Opened 5 years ago
Closed 5 years ago
#19533 closed enhancement (fixed)
Add a method to find a vector x such that Q(x) = C, where Q is a quadratic form and C is a constant.
Reported by:  tgaona  Owned by:  tgaona 

Priority:  minor  Milestone:  sage6.10 
Component:  quadratic forms  Keywords:  quadratic forms 
Cc:  annahaensch  Merged in:  
Authors:  Tyler Gaona  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  c3ef1ed (Commits, GitHub, GitLab)  Commit:  c3ef1ed9ae3cfece81f4587b3f824e8ba115f2ec 
Dependencies:  Stopgaps: 
Description
This algorithm can be implemented using PARI's qfsolve(). Will be used as a component for the method implemented in Ticket 19112
Change History (12)
comment:1 Changed 5 years ago by
 Branch set to u/tgaona/ticket/19533
comment:2 Changed 5 years ago by
 Commit set to cfc69b209bef0e1db4c4238c19db68c3012684f8
 Status changed from new to needs_review
comment:3 Changed 5 years ago by
One quick comment (I have not fully checked the patch): it would be useful to change the function to
def solve(self, c=0)
such that Q.solve()
without argument finds a zero of Q
. Then adjust the documentation and add a test for this case.
comment:4 Changed 5 years ago by
 Status changed from needs_review to needs_work
Why divide by abs(z)
instead of z
? I'm not saying it's wrong, there is just no reason for it.
You should limit the length of lines to 72 characters if possible, so wrap this comment:
# Case 2: We found a solution self(x) = 0. Let e be any vector such that B(x,e) != 0, where B is the bilinear form corresponding to self. # To find e, just try all unit vectors (0,..0,1,0...0). Let a = (c  self(e))/2*B(x,e) and let y = e + a*x. # Then self(y) = B(e + a*x, e + a*x) = self(e) + 2B(e, a*x) = self(e) + 2([c  self(e)]/[2B(x,e)]) * B(x,e) = c.
comment:5 Changed 5 years ago by
Another minor thing: I would change the error message to
raise ArithmeticError("no solution found (local obstruction at {})".format(x))
comment:6 Changed 5 years ago by
Replace
if z != 0:
by
if z:
comment:7 Changed 5 years ago by
 Commit changed from cfc69b209bef0e1db4c4238c19db68c3012684f8 to cb3f891f66da3e2ecb13b6c08a0bfc40afff4882
Branch pushed to git repo; I updated commit sha1. New commits:
cb3f891  adds default parameter to solve(). minor formatting changes.

comment:8 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:9 Changed 5 years ago by
 Branch changed from u/tgaona/ticket/19533 to u/jdemeyer/ticket/19533
comment:10 Changed 5 years ago by
 Commit changed from cb3f891f66da3e2ecb13b6c08a0bfc40afff4882 to c3ef1ed9ae3cfece81f4587b3f824e8ba115f2ec
 Reviewers set to Jeroen Demeyer
I have made a lot of small changes. I also added a few more examples. If you agree, you can set this to positive_review.
New commits:
c3ef1ed  Further fixes to quadratic form solver

comment:11 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:12 Changed 5 years ago by
 Branch changed from u/jdemeyer/ticket/19533 to c3ef1ed9ae3cfece81f4587b3f824e8ba115f2ec
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Adds the `solve` method for quadratic forms.