Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
sagemathgh-36528: some cleanup in quadratic forms
    
mostly a pep8 cleanup in quadratic_forms, also some code details

hand-made

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
    
URL: sagemath#36528
Reported by: Frédéric Chapoton
Reviewer(s): David Coudert
  • Loading branch information
Release Manager committed Oct 29, 2023
2 parents 5fdb3bc + bf64a65 commit 52dacd3
Show file tree
Hide file tree
Showing 12 changed files with 145 additions and 147 deletions.
10 changes: 6 additions & 4 deletions src/sage/quadratic_forms/binary_qf.py
Original file line number Diff line number Diff line change
Expand Up @@ -910,7 +910,8 @@ def reduced_form(self, transformation=False, algorithm="default"):
if algorithm == 'sage':
if self.discriminant() <= 0:
raise NotImplementedError('reduction of definite binary '
'quadratic forms is not implemented in Sage')
'quadratic forms is not implemented '
'in Sage')
return self._reduce_indef(transformation)

elif algorithm == 'pari':
Expand Down Expand Up @@ -1154,14 +1155,15 @@ def cycle(self, proper=False):
if self.discriminant().is_square():
# Buchmann/Vollmer assume the discriminant to be non-square
raise NotImplementedError('computation of cycles is only '
'implemented for non-square discriminants')
'implemented for non-square '
'discriminants')
if proper:
# Prop 6.10.5 in Buchmann Vollmer
C = list(self.cycle(proper=False)) # make a copy that we can modify
if len(C) % 2:
C += C
for i in range(len(C)//2):
C[2*i+1] = C[2*i+1]._Tau()
for i in range(len(C) // 2):
C[2 * i + 1] = C[2 * i + 1]._Tau()
return C
if not hasattr(self, '_cycle_list'):
C = [self]
Expand Down
7 changes: 3 additions & 4 deletions src/sage/quadratic_forms/genera/all.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
#*****************************************************************************
# ****************************************************************************
# Copyright (C) 2007 David Kohel <[email protected]>
#
# Distributed under the terms of the GNU General Public License (GPL)
#
# http://www.gnu.org/licenses/
#*****************************************************************************

# https://www.gnu.org/licenses/
# ****************************************************************************
from .genus import Genus, LocalGenusSymbol, is_GlobalGenus
9 changes: 4 additions & 5 deletions src/sage/quadratic_forms/quadratic_form__automorphisms.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ def short_vector_list_up_to_length(self, len_bound, up_to_sign_flag=False):
A list of lists of vectors such that entry `[i]` contains all
vectors of length `i`.
EXAMPLES::
sage: Q = DiagonalQuadraticForm(ZZ, [1,3,5,7])
Expand Down Expand Up @@ -143,13 +142,13 @@ def short_vector_list_up_to_length(self, len_bound, up_to_sign_flag=False):
[],
[(0, 1, 0, 0)],
[(1, 1, 0, 0), (1, -1, 0, 0), (2, 0, 0, 0)]]
sage: Q = QuadraticForm(matrix(6, [2, 1, 1, 1, -1, -1, 1, 2, 1, 1, -1, -1, 1, 1, 2, 0, -1, -1, 1, 1, 0, 2, 0, -1, -1, -1, -1, 0, 2, 1, -1, -1, -1, -1, 1, 2]))
sage: m6 = matrix(6, [2, 1, 1, 1, -1, -1, 1, 2, 1, 1, -1, -1,
....: 1, 1, 2, 0, -1, -1, 1, 1, 0, 2, 0, -1,
....: -1, -1, -1, 0, 2, 1, -1, -1, -1, -1, 1, 2])
sage: Q = QuadraticForm(m6)
sage: vs = Q.short_vector_list_up_to_length(8)
sage: [len(vs[i]) for i in range(len(vs))]
[1, 72, 270, 720, 936, 2160, 2214, 3600]
sage: vs = Q.short_vector_list_up_to_length(30) # long time (28s on sage.math, 2014)
sage: [len(vs[i]) for i in range(len(vs))] # long time
[1, 72, 270, 720, 936, 2160, 2214, 3600, 4590, 6552, 5184, 10800, 9360, 12240, 13500, 17712, 14760, 25920, 19710, 26064, 28080, 36000, 25920, 47520, 37638, 43272, 45900, 59040, 46800, 75600]
The cases of ``len_bound < 2`` led to exception or infinite runtime before.
Expand Down
6 changes: 3 additions & 3 deletions src/sage/quadratic_forms/quadratic_form__count_local_2.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ def count_congruence_solutions_as_vector(self, p, k, m, zvec, nzvec):
return CountAllLocalTypesNaive(self, p, k, m, zvec, nzvec)


##///////////////////////////////////////////
##/// Front-ends for our counting routines //
##///////////////////////////////////////////
# //////////////////////////////////////////
# // Front-ends for our counting routines //
# //////////////////////////////////////////

def count_congruence_solutions(self, p, k, m, zvec, nzvec):
r"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ def has_equivalent_Jordan_decomposition_at_prime(self, other, p):
# Check O'Meara's condition (ii) when appropriate
if norm_list[i + 1] % (4 * norm_list[i]) == 0:
if self_hasse_chain_list[i] * hilbert_symbol(norm_list[i] * other_chain_det_list[i], -self_chain_det_list[i], 2) \
!= other_hasse_chain_list[i] * hilbert_symbol(norm_list[i], -other_chain_det_list[i], 2): # Nipp conditions
!= other_hasse_chain_list[i] * hilbert_symbol(norm_list[i], -other_chain_det_list[i], 2): # Nipp conditions
return False

# All tests passed for the prime 2.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@
quadratic forms over the rationals.
"""

#*****************************************************************************
# ****************************************************************************
# Copyright (C) 2007 William Stein and Jonathan Hanke
# Copyright (C) 2015 Jeroen Demeyer <[email protected]>
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 2 of the License, or
# (at your option) any later version.
# http://www.gnu.org/licenses/
#*****************************************************************************
# https://www.gnu.org/licenses/
# ****************************************************************************

###########################################################################
# TO DO: Add routines for hasse invariants at all places, anisotropic
Expand Down
40 changes: 20 additions & 20 deletions src/sage/quadratic_forms/quadratic_form__mass.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,27 @@

# Import all general mass finding routines
from sage.quadratic_forms.quadratic_form__mass__Siegel_densities import \
mass__by_Siegel_densities, \
Pall_mass_density_at_odd_prime, \
Watson_mass_at_2, \
Kitaoka_mass_at_2, \
mass_at_two_by_counting_mod_power
mass__by_Siegel_densities, \
Pall_mass_density_at_odd_prime, \
Watson_mass_at_2, \
Kitaoka_mass_at_2, \
mass_at_two_by_counting_mod_power
from sage.quadratic_forms.quadratic_form__mass__Conway_Sloane_masses import \
parity, \
is_even, \
is_odd, \
conway_species_list_at_odd_prime, \
conway_species_list_at_2, \
conway_octane_of_this_unimodular_Jordan_block_at_2, \
conway_diagonal_factor, \
conway_cross_product_doubled_power, \
conway_type_factor, \
conway_p_mass, \
conway_standard_p_mass, \
conway_standard_mass, \
conway_mass
# conway_generic_mass, \
# conway_p_mass_adjustment
parity, \
is_even, \
is_odd, \
conway_species_list_at_odd_prime, \
conway_species_list_at_2, \
conway_octane_of_this_unimodular_Jordan_block_at_2, \
conway_diagonal_factor, \
conway_cross_product_doubled_power, \
conway_type_factor, \
conway_p_mass, \
conway_standard_p_mass, \
conway_standard_mass, \
conway_mass
# conway_generic_mass, \
# conway_p_mass_adjustment

###################################################

Expand Down
3 changes: 1 addition & 2 deletions src/sage/quadratic_forms/quadratic_form__neighbors.py
Original file line number Diff line number Diff line change
Expand Up @@ -389,8 +389,7 @@ def orbits_lines_mod_p(self, p):
# but in gap we act from the right!! --> transpose
gens = self.automorphism_group().gens()
gens = [g.matrix().transpose().change_ring(GF(p)) for g in gens]
orbs = libgap.function_factory(
"""function(gens, p)
orbs = libgap.function_factory("""function(gens, p)
local one, G, reps, V, n, orb;
one:= One(GF(p));
G:=Group(List(gens, g -> g*one));
Expand Down
11 changes: 6 additions & 5 deletions src/sage/quadratic_forms/quadratic_form__siegel_product.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,13 @@ def siegel_product(self, u):
verbose(" u = " + str(u) + "\n")

# Make the odd generic factors
if ((n % 2) == 1):
m = (n-1) // 2
if n % 2:
m = (n - 1) // 2
d1 = fundamental_discriminant(((-1)**m) * 2*d * u) # Replaced d by 2d here to compensate for the determinant
f = abs(d1) # gaining an odd power of 2 by using the matrix of 2Q instead
# of the matrix of Q.
# --> Old d1 = CoreDiscriminant((mpz_class(-1)^m) * d * u);
f = abs(d1)
# gaining an odd power of 2 by using the matrix of 2Q instead
# of the matrix of Q.
# --> Old d1 = CoreDiscriminant((mpz_class(-1)^m) * d * u);

# Make the ratio of factorials factor: [(2m)! / m!] * prod_{i=1}^m (2*i-1)
factor1 = 1
Expand Down
51 changes: 24 additions & 27 deletions src/sage/quadratic_forms/quadratic_form__split_local_covering.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
from sage.rings.real_double import RDF
from sage.matrix.matrix_space import MatrixSpace
from sage.matrix.constructor import matrix
from sage.misc.lazy_import import lazy_import
lazy_import("sage.functions.all", "floor")
from sage.rings.integer_ring import ZZ
from sage.arith.misc import GCD

Expand Down Expand Up @@ -94,19 +92,19 @@ def cholesky_decomposition(self, bit_prec=53):

# 2. Loop on i
for i in range(n):
for j in range(i+1, n):
Q[j,i] = Q[i,j] # Is this line redundant?
Q[i,j] = Q[i,j] / Q[i,i]
for j in range(i + 1, n):
Q[j, i] = Q[i, j] # Is this line redundant?
Q[i, j] = Q[i, j] / Q[i, i]

# 3. Main Loop
for k in range(i+1, n):
for k in range(i + 1, n):
for l in range(k, n):
Q[k,l] = Q[k,l] - Q[k,i] * Q[i,l]
Q[k, l] = Q[k, l] - Q[k, i] * Q[i, l]

# 4. Zero out the strictly lower-triangular entries
for i in range(n):
for j in range(i):
Q[i,j] = 0
Q[i, j] = 0

return Q

Expand Down Expand Up @@ -178,7 +176,7 @@ def vectors_by_length(self, bound):
# but if eps is too small, roundoff errors may knock off some
# vectors of norm = bound (see #7100)
eps = RDF(1e-6)
bound = ZZ(floor(max(bound, 0)))
bound = ZZ(max(bound, 0)) # bound is an integer
Theta_Precision = bound + eps
n = self.dim()

Expand All @@ -192,7 +190,7 @@ def vectors_by_length(self, bound):
# 1. Initialize
T = n * [RDF(0)] # Note: We index the entries as 0 --> n-1
U = n * [RDF(0)]
i = n-1
i = n - 1
T[i] = RDF(Theta_Precision)
U[i] = RDF(0)

Expand All @@ -201,27 +199,27 @@ def vectors_by_length(self, bound):

# 2. Compute bounds
Z = (T[i] / Q[i][i]).sqrt(extend=False)
L[i] = ( Z - U[i]).floor()
L[i] = (Z - U[i]).floor()
x[i] = (-Z - U[i]).ceil()

done_flag = False
Q_val = 0 # WARNING: Still need a good way of checking overflow for this value...
Q_val = 0 # WARNING: Still need a good way of checking overflow for this value...

# Big loop which runs through all vectors
while not done_flag:

# 3b. Main loop -- try to generate a complete vector x (when i=0)
while (i > 0):
T[i-1] = T[i] - Q[i][i] * (x[i] + U[i]) * (x[i] + U[i])
T[i - 1] = T[i] - Q[i][i] * (x[i] + U[i]) * (x[i] + U[i])
i = i - 1
U[i] = 0
for j in range(i+1, n):
for j in range(i + 1, n):
U[i] = U[i] + Q[i][j] * x[j]

# Now go back and compute the bounds...
# 2. Compute bounds
Z = (T[i] / Q[i][i]).sqrt(extend=False)
L[i] = ( Z - U[i]).floor()
L[i] = (Z - U[i]).floor()
x[i] = (-Z - U[i]).ceil()

# carry if we go out of bounds -- when Z is so small that
Expand All @@ -241,20 +239,20 @@ def vectors_by_length(self, bound):
print(" Float = ", Q_val_double, " Long = ", Q_val)
raise RuntimeError("The roundoff error is bigger than 0.001, so we should use more precision somewhere...")

if (Q_val <= bound):
if Q_val <= bound:
theta_vec[Q_val].append(deepcopy(x))

# 5. Check if x = 0, for exit condition. =)
j = 0
done_flag = True
while (j < n):
if (x[j] != 0):
while j < n:
if x[j] != 0:
done_flag = False
j += 1

# 3a. Increment (and carry if we go out of bounds)
x[i] += 1
while (x[i] > L[i]) and (i < n-1):
while i < n - 1 and x[i] > L[i]:
i += 1
x[i] += 1

Expand Down Expand Up @@ -284,7 +282,6 @@ def complementary_subform_to_vector(self, v):
OUTPUT: a :class:`QuadraticForm` over `\ZZ`
EXAMPLES::
sage: Q1 = DiagonalQuadraticForm(ZZ, [1,3,5,7])
Expand Down Expand Up @@ -317,7 +314,7 @@ def complementary_subform_to_vector(self, v):

# Find the first non-zero component of v, and call it nz (Note: 0 <= nz < n)
nz = 0
while (nz < n) and (v[nz] == 0):
while nz < n and v[nz] == 0:
nz += 1

# Abort if v is the zero vector
Expand All @@ -334,27 +331,27 @@ def complementary_subform_to_vector(self, v):
d = Q1[0, 0]

# For each row/column, perform elementary operations to cancel them out.
for i in range(1,n):
for i in range(1, n):

# Check if the (i,0)-entry is divisible by d,
# and stretch its row/column if not.
if Q1[i,0] % d != 0:
Q1 = Q1.multiply_variable(d / GCD(d, Q1[i, 0]//2), i)
if Q1[i, 0] % d:
Q1 = Q1.multiply_variable(d / GCD(d, Q1[i, 0] // 2), i)

# Now perform the (symmetric) elementary operations to cancel out the (i,0) entries/
Q1 = Q1.add_symmetric(-(Q1[i,0]/2) / (GCD(d, Q1[i,0]//2)), i, 0)
Q1 = Q1.add_symmetric(-(Q1[i, 0] // 2) / (GCD(d, Q1[i, 0] // 2)), i, 0)

# Check that we're done!
done_flag = True
for i in range(1, n):
if Q1[0,i] != 0:
if Q1[0, i] != 0:
done_flag = False

if not done_flag:
raise RuntimeError("There is a problem cancelling out the matrix entries! =O")

# Return the complementary matrix
return Q1.extract_variables(range(1,n))
return Q1.extract_variables(range(1, n))


def split_local_cover(self):
Expand Down
Loading

0 comments on commit 52dacd3

Please sign in to comment.