Skip to content

Commit 6b76b79

Browse files
author
Release Manager
committed
sagemathgh-41012: Fix qfsolve see the newly added doctest, they previously fail. this is a combination of two bugs * pari qfsolve function may return a matrix, but the original code does not handle this case * after finding a solution x, it's possible that there's no vector e such that B(x, e) != 0. In this case for any vector v, self(v) = self(v+λx), therefore it suffice to find solution in some subspace complement to x, if there exists a solution then there must exist one in that subspace by adding appropriate multiple of x. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] The title is concise and informative. - [ ] 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 and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#41012 Reported by: user202729 Reviewer(s): grnx, user202729
2 parents 87ec7fe + 8c3442a commit 6b76b79

File tree

1 file changed

+68
-21
lines changed

1 file changed

+68
-21
lines changed

src/sage/quadratic_forms/qfsolve.py

Lines changed: 68 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -31,23 +31,17 @@
3131
from sage.rings.rational_field import QQ
3232
from sage.rings.integer import Integer
3333
from sage.modules.free_module_element import vector
34-
from sage.matrix.constructor import Matrix
34+
from sage.matrix.constructor import matrix
35+
from sage.structure.element import Matrix
3536

3637

3738
def qfsolve(G):
3839
r"""
3940
Find a solution `x = (x_0,...,x_n)` to `x G x^t = 0` for an
4041
`n \times n`-matrix ``G`` over `\QQ`.
4142
42-
OUTPUT:
43-
44-
If a solution exists, return a vector of rational numbers `x`.
45-
Otherwise, returns `-1` if no solution exists over the reals or a
46-
prime `p` if no solution exists over the `p`-adic field `\QQ_p`.
47-
48-
ALGORITHM:
49-
50-
Uses PARI/GP function :pari:`qfsolve`.
43+
OUTPUT: May return an integer, a vector or a matrix.
44+
The output format is identical to that of :pari:`qfsolve`.
5145
5246
EXAMPLES::
5347
@@ -74,11 +68,26 @@ def qfsolve(G):
7468
sage: M = Matrix(QQ, [[3, 0, 0, 0], [0, 5, 0, 0], [0, 0, -7, 0], [0, 0, 0, -11]])
7569
sage: qfsolve(M)
7670
(3, 4, -3, -2)
71+
72+
sage: M = Matrix(QQ, [[0, 0], [0, 0]])
73+
sage: ret = qfsolve(M); ret
74+
[1 0]
75+
[0 1]
76+
sage: ret.parent()
77+
Full MatrixSpace of 2 by 2 dense matrices over Rational Field
78+
79+
sage: M = Matrix(QQ, [[1, 0], [0, -2]])
80+
sage: ret = qfsolve(M); ret
81+
-2
82+
sage: ret.parent()
83+
Integer Ring
7784
"""
7885
ret = G.__pari__().qfsolve()
7986
if ret.type() == 't_COL':
8087
return vector(QQ, ret)
81-
return ZZ(ret)
88+
if ret.type() == 't_MAT':
89+
return ret.sage().change_ring(QQ)
90+
return ret.sage()
8291

8392

8493
def qfparam(G, sol):
@@ -133,7 +142,8 @@ def solve(self, c=0):
133142
134143
ALGORITHM:
135144
136-
Uses PARI's :pari:`qfsolve`. Algorithm described by Jeroen Demeyer; see comments on :issue:`19112`
145+
Uses PARI's :pari:`qfsolve`, with an extension to handle the case ``c != 0``
146+
described by Jeroen Demeyer in :issue:`19112`.
137147
138148
EXAMPLES::
139149
@@ -158,7 +168,7 @@ def solve(self, c=0):
158168
sage: F.solve()
159169
Traceback (most recent call last):
160170
...
161-
ArithmeticError: no solution found (local obstruction at -1)
171+
ArithmeticError: no solution found (local obstruction at RR)
162172
163173
::
164174
@@ -203,32 +213,61 @@ def solve(self, c=0):
203213
Traceback (most recent call last):
204214
...
205215
TypeError: solving quadratic forms is only implemented over QQ
216+
217+
TESTS::
218+
219+
sage: R.<x1,x2,x3> = QQ[]
220+
sage: QuadraticForm(x3^2).solve(9)
221+
(0, 0, 3)
222+
sage: QuadraticForm(R.zero()).solve(9)
223+
Traceback (most recent call last):
224+
...
225+
ArithmeticError: no solution found (local obstruction at RR)
226+
sage: DiagonalQuadraticForm(QQ, [1, -2]).solve()
227+
Traceback (most recent call last):
228+
...
229+
ArithmeticError: no solution found (local obstruction at some prime factor of det(self.matrix()))
206230
"""
231+
def check_obstruction(x):
232+
"""
233+
Local helper. ``x`` is the return value of ``qfsolve``.
234+
Raise an exception if ``x`` indicates an obstruction,
235+
otherwise return ``x``.
236+
"""
237+
if isinstance(x, Integer):
238+
if x == -1:
239+
x = "RR"
240+
elif x == -2:
241+
# we faithfully report what pari tells us
242+
# it is likely pari doesn't report the prime is because it requires factorizing the determinant
243+
x = "some prime factor of det(self.matrix())"
244+
raise ArithmeticError(f"no solution found (local obstruction at {x})")
245+
return x
246+
207247
if self.base_ring() is not QQ:
208248
raise TypeError("solving quadratic forms is only implemented over QQ")
209249

210250
M = self.Gram_matrix()
211251

212252
# If no argument passed for c, we just pass self into qfsolve().
213253
if not c:
214-
x = qfsolve(M)
215-
if isinstance(x, Integer):
216-
raise ArithmeticError(f"no solution found (local obstruction at {x})")
254+
x = check_obstruction(qfsolve(M))
255+
if isinstance(x, Matrix):
256+
return x.column(0)
217257
return x
218258

219259
# If c != 0, define a new quadratic form Q = self - c*z^2
220260
d = self.dim()
221-
N = Matrix(self.base_ring(), d+1, d+1)
261+
N = matrix(self.base_ring(), d+1, d+1)
222262
for i in range(d):
223263
for j in range(d):
224264
N[i, j] = M[i, j]
225265
N[d, d] = -c
226266

227267
# Find a solution x to Q(x) = 0, using qfsolve()
228-
x = qfsolve(N)
229-
# Raise an error if qfsolve() doesn't find a solution
230-
if isinstance(x, Integer):
231-
raise ArithmeticError(f"no solution found (local obstruction at {x})")
268+
x = check_obstruction(qfsolve(N))
269+
if isinstance(x, Matrix):
270+
x = x.column(0)
232271

233272
# Let z be the last term of x, and remove z from x
234273
z = x[-1]
@@ -248,6 +287,14 @@ def solve(self, c=0):
248287
while self.bilinear_map(x, e) == 0:
249288
e[i] = 0
250289
i += 1
290+
if i >= d:
291+
# Restrict the quadratic form to a subspace complement to x, then solve recursively
292+
# note that complementary_subform_to_vector returns the orthogonal (not necessary complement)
293+
# subspace with respect to self, which is not what we want
294+
i = next(i for i in range(d) if x[i])
295+
from sage.quadratic_forms.quadratic_form import QuadraticForm
296+
x = QuadraticForm(self.matrix().delete_rows([0]).delete_columns([0])).solve(c)
297+
return vector([*x[:i], 0, *x[i:]])
251298
e[i] = 1
252299

253300
a = (c - self(e)) / (2 * self.bilinear_map(x, e))

0 commit comments

Comments
 (0)