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: | sage-6.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.