Skip to content

Commit 5b5adf1

Browse files
author
Release Manager
committed
gh-36035: cython-lint : further fixes in quadratic forms <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> This fixes some suggestions of `cython-lint` in `quadratic_forms`. <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes #12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - #12345: short description why this is a dependency - #34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: #36035 Reported by: Frédéric Chapoton Reviewer(s): Frédéric Chapoton, Matthias Köppe
2 parents 5340692 + beb071d commit 5b5adf1

File tree

2 files changed

+61
-74
lines changed

2 files changed

+61
-74
lines changed

src/sage/quadratic_forms/count_local_2.pyx

Lines changed: 37 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ r"""
22
Optimized counting of congruence solutions
33
"""
44
from sage.arith.misc import is_prime, kronecker as kronecker_symbol, valuation
5-
from sage.rings.finite_rings.integer_mod cimport IntegerMod_gmp
65
from sage.rings.finite_rings.integer_mod import Mod
76
from sage.rings.finite_rings.integer_mod_ring import IntegerModRing
87

@@ -73,16 +72,16 @@ def count_modp__by_gauss_sum(n, p, m, Qdet):
7372

7473
# Compute the Gauss sum
7574
neg1 = -1
76-
if not (m % p):
75+
if not m % p:
7776
if n % 2:
78-
count = (p**(n-1))
77+
count = p**(n-1)
7978
else:
80-
count = (p**(n-1)) + (p-1) * (p**((n-2)/2)) * kronecker_symbol(((neg1**(n/2)) * Qdet) % p, p)
79+
count = p**(n-1) + (p-1) * (p**((n-2)//2)) * kronecker_symbol(((neg1**(n//2)) * Qdet) % p, p)
8180
else:
8281
if n % 2:
83-
count = (p**(n-1)) + (p**((n-1)/2)) * kronecker_symbol(((neg1**((n-1)/2)) * Qdet * m) % p, p)
82+
count = p**(n-1) + p**((n-1)//2) * kronecker_symbol(((neg1**((n-1)//2)) * Qdet * m) % p, p)
8483
else:
85-
count = (p**(n-1)) - (p**((n-2)/2)) * kronecker_symbol(((neg1**(n/2)) * Qdet) % p, p)
84+
count = p**(n-1) - p**((n-2)//2) * kronecker_symbol(((neg1**(n//2)) * Qdet) % p, p)
8685

8786
# Return the result
8887
return count
@@ -103,11 +102,6 @@ cdef CountAllLocalTypesNaive_cdef(Q, p, k, m, zvec, nzvec):
103102
R = p ** k
104103
Q1 = Q.change_ring(IntegerModRing(R))
105104

106-
# Cython Variables
107-
cdef IntegerMod_gmp zero, one
108-
zero = IntegerMod_gmp(IntegerModRing(R), 0)
109-
one = IntegerMod_gmp(IntegerModRing(R), 1)
110-
111105
# Initialize the counting vector
112106
count_vector = [0 for i in range(6)]
113107

@@ -119,7 +113,7 @@ cdef CountAllLocalTypesNaive_cdef(Q, p, k, m, zvec, nzvec):
119113
m1 = Mod(m, R)
120114

121115
# Count the local solutions
122-
for i from 0 <= i < R_n:
116+
for i in range(R_n):
123117

124118
# Perform a carry (when value = R-1) until we can increment freely
125119
ptr = len(v)
@@ -128,20 +122,20 @@ cdef CountAllLocalTypesNaive_cdef(Q, p, k, m, zvec, nzvec):
128122
ptr += -1
129123

130124
# Only increment if we're not already at the zero vector =)
131-
if (ptr > 0):
125+
if ptr > 0:
132126
v[ptr-1] += 1
133127

134128
# Evaluate Q(v) quickly
135129
tmp_val = Mod(0, R)
136130
for a from 0 <= a < n:
137131
for b from a <= b < n:
138-
tmp_val += Q1[a,b] * v[a] * v[b]
132+
tmp_val += Q1[a, b] * v[a] * v[b]
139133

140134
# Sort the solution by it's type
141-
#if (Q1(v) == m1):
142-
if (tmp_val == m1):
135+
# if Q1(v) == m1:
136+
if tmp_val == m1:
143137
solntype = local_solution_type_cdef(Q1, p, v, zvec, nzvec)
144-
if (solntype != 0):
138+
if solntype != 0:
145139
count_vector[solntype] += 1
146140

147141
# Generate the Bad-type and Total counts
@@ -187,11 +181,6 @@ def CountAllLocalTypesNaive(Q, p, k, m, zvec, nzvec):
187181
return CountAllLocalTypesNaive_cdef(Q, p, k, m, zvec, nzvec)
188182

189183

190-
191-
192-
193-
194-
195184
cdef local_solution_type_cdef(Q, p, w, zvec, nzvec):
196185
"""
197186
Internal routine to check if a given solution vector `w` (of `Q(w) =
@@ -209,91 +198,87 @@ cdef local_solution_type_cdef(Q, p, w, zvec, nzvec):
209198

210199
# Check if the solution satisfies the zvec "zero" congruence conditions
211200
# (either zvec is empty or its components index the zero vector mod p)
212-
if (zvec is None) or (not zvec):
201+
if zvec is None or not zvec:
213202
zero_flag = True
214203
else:
215204
zero_flag = False
216205
i = 0
217-
while ( (i < len(zvec)) and ((w[zvec[i]] % p) == 0) ): # Increment so long as our entry is zero (mod p)
206+
while i < len(zvec) and not w[zvec[i]] % p: # Increment so long as our entry is zero (mod p)
218207
i += 1
219-
if (i == len(zvec)): # If we make it through all entries then the solution is zero (mod p)
208+
if i == len(zvec): # If we make it through all entries then the solution is zero (mod p)
220209
zero_flag = True
221210

222-
223211
# DIAGNOSTIC
224-
#print("IsLocalSolutionType: Finished the Zero congruence condition test \n")
212+
# print("IsLocalSolutionType: Finished the Zero congruence condition test \n")
225213

226214
if not zero_flag:
227215
return <long> 0
228216

229217
# DIAGNOSTIC
230-
#print("IsLocalSolutionType: Passed the Zero congruence condition test \n")
218+
# print("IsLocalSolutionType: Passed the Zero congruence condition test \n")
231219

232220
# Check if the solution satisfies the nzvec "nonzero" congruence conditions
233221
# (nzvec is non-empty and its components index a non-zero vector mod p)
234-
if (nzvec is None):
222+
if nzvec is None:
235223
nonzero_flag = True
236-
elif (len(nzvec) == 0):
224+
elif len(nzvec) == 0:
237225
nonzero_flag = False # Trivially no solutions in this case!
238226
else:
239227
nonzero_flag = False
240228
i = 0
241-
while ((not nonzero_flag) and (i < len(nzvec))):
242-
if ((w[nzvec[i]] % p) != 0):
229+
while not nonzero_flag and i < len(nzvec):
230+
if w[nzvec[i]] % p:
243231
nonzero_flag = True # The non-zero condition is satisfied when we find one non-zero entry
244232
i += 1
245233

246234
if not nonzero_flag:
247235
return <long> 0
248236

249-
250237
# Check if the solution has the appropriate (local) type:
251238
# -------------------------------------------------------
252239

253240
# 1: Check Good-type
254-
for i from 0 <= i < n:
255-
if (((w[i] % p) != 0) and ((Q[i,i] % p) != 0)):
241+
for i in range(n):
242+
if w[i] % p and Q[i, i] % p:
256243
return <long> 1
257-
if (p == 2):
258-
for i from 0 <= i < (n - 1):
259-
if (((Q[i,i+1] % p) != 0) and (((w[i] % p) != 0) or ((w[i+1] % p) != 0))):
244+
if p == 2:
245+
for i in range(n - 1):
246+
if Q[i, i+1] % p and (w[i] % p or w[i+1] % p):
260247
return <long> 1
261248

262-
263249
# 2: Check Zero-type
264250
Zero_flag = True
265-
for i from 0 <= i < n:
266-
if ((w[i] % p) != 0):
251+
for i in range(n):
252+
if w[i] % p:
267253
Zero_flag = False
268254
if Zero_flag:
269255
return <long> 2
270256

271-
272257
# Check if wS1 is zero or not
273258
wS1_nonzero_flag = False
274259
for i from 0 <= i < n:
275260

276261
# Compute the valuation of each index, allowing for off-diagonal terms
277-
if (Q[i,i] == 0):
278-
if (i == 0):
279-
val = valuation(Q[i,i+1], p) # Look at the term to the right
280-
elif (i == n - 1):
281-
val = valuation(Q[i-1,i], p) # Look at the term above
262+
if Q[i, i] == 0:
263+
if i == 0:
264+
val = valuation(Q[i, i+1], p) # Look at the term to the right
265+
elif i == n - 1:
266+
val = valuation(Q[i-1, i], p) # Look at the term above
282267
else:
283-
val = valuation(Q[i,i+1] + Q[i-1,i], p) # Finds the valuation of the off-diagonal term since only one isn't zero
268+
val = valuation(Q[i, i+1] + Q[i-1, i], p) # Finds the valuation of the off-diagonal term since only one isn't zero
284269
else:
285-
val = valuation(Q[i,i], p)
270+
val = valuation(Q[i, i], p)
286271

287272
# Test each index
288-
if ((val == 1) and ((w[i] % p) != 0)):
273+
if val == 1 and w[i] % p:
289274
wS1_nonzero_flag = True
290275

291276
# 4: Check Bad-type I
292-
if (wS1_nonzero_flag is True):
277+
if wS1_nonzero_flag:
293278
return <long> 4
294279

295280
# 5: Check Bad-type II
296-
if (wS1_nonzero_flag is False):
281+
if not wS1_nonzero_flag:
297282
return <long> 5
298283

299284
# Error if we get here! =o

src/sage/quadratic_forms/quadratic_form__evaluate.pyx

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,15 @@
44
def QFEvaluateVector(Q, v):
55
r"""
66
Evaluate this quadratic form `Q` on a vector or matrix of elements
7-
coercible to the base ring of the quadratic form. If a vector
8-
is given, then the output will be the ring element `Q(v)`, but if a
9-
matrix is given, then the output will be the quadratic form `Q'`
10-
which in matrix notation is given by:
7+
coercible to the base ring of the quadratic form.
8+
9+
If a vector is given, then the output will be the ring element
10+
`Q(v)`, but if a matrix is given, then the output will be the
11+
quadratic form `Q'` which in matrix notation is given by:
1112
1213
.. MATH::
1314
14-
Q' = v^t\cdot Q\cdot v.
15+
Q' = v^t\cdot Q\cdot v.
1516
1617
.. NOTE::
1718
@@ -43,7 +44,6 @@ def QFEvaluateVector(Q, v):
4344
return QFEvaluateVector_cdef(Q, v)
4445

4546

46-
4747
cdef QFEvaluateVector_cdef(Q, v):
4848
r"""
4949
Routine to quickly evaluate a quadratic form `Q` on a vector `v`. See
@@ -54,16 +54,15 @@ cdef QFEvaluateVector_cdef(Q, v):
5454
# (In matrix notation: A^t * Q * A)
5555
n = Q.dim()
5656

57-
tmp_val = Q.base_ring()(0)
58-
for i from 0 <= i < n:
59-
for j from i <= j < n:
60-
tmp_val += Q[i,j] * v[i] * v[j]
57+
tmp_val = Q.base_ring().zero()
58+
for i in range(n):
59+
for j in range(i, n):
60+
tmp_val += Q[i, j] * v[i] * v[j]
6161

6262
# Return the value (over R)
6363
return Q.base_ring().coerce(tmp_val)
6464

6565

66-
6766
def QFEvaluateMatrix(Q, M, Q2):
6867
r"""
6968
Evaluate this quadratic form `Q` on a matrix `M` of elements coercible
@@ -72,7 +71,7 @@ def QFEvaluateMatrix(Q, M, Q2):
7271
7372
.. MATH::
7473
75-
Q_2 = M^t\cdot Q\cdot M.
74+
Q_2 = M^t\cdot Q\cdot M.
7675
7776
.. NOTE::
7877
@@ -125,14 +124,17 @@ cdef QFEvaluateMatrix_cdef(Q, M, Q2):
125124
# TODO: Check the dimensions of M are compatible with those of Q and Q2
126125

127126
# Evaluate Q(M) into Q2
128-
for k from 0 <= k < m:
129-
for l from k <= l < m:
130-
tmp_sum = Q2.base_ring()(0)
131-
for i from 0 <= i < n:
132-
for j from i <= j < n:
133-
if (k == l):
134-
tmp_sum += Q[i,j] * (M[i,k] * M[j,l])
135-
else:
136-
tmp_sum += Q[i,j] * (M[i,k] * M[j,l] + M[i,l] * M[j,k])
137-
Q2[k,l] = tmp_sum
127+
for k in range(m):
128+
for l in range(k, m):
129+
tmp_sum = Q2.base_ring().zero()
130+
if k == l:
131+
for i in range(n):
132+
for j in range(i, n):
133+
tmp_sum += Q[i, j] * (M[i, k] * M[j, l])
134+
else:
135+
for i in range(n):
136+
for j in range(i, n):
137+
tmp_sum += Q[i, j] * (M[i, k] * M[j, l] +
138+
M[i, l] * M[j, k])
139+
Q2[k, l] = tmp_sum
138140
return Q2

0 commit comments

Comments
 (0)