Opened 4 years ago

Closed 4 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) 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 4 years ago by tgaona

  • Branch set to u/tgaona/ticket/19533

comment:2 Changed 4 years ago by tgaona

  • Commit set to cfc69b209bef0e1db4c4238c19db68c3012684f8
  • Status changed from new to needs_review

New commits:

cfc69b2Adds the `solve` method for quadratic forms.

comment:3 Changed 4 years ago by jdemeyer

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 4 years ago by jdemeyer

  • 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 4 years ago by jdemeyer

Another minor thing: I would change the error message to

raise ArithmeticError("no solution found (local obstruction at {})".format(x))

comment:6 Changed 4 years ago by jdemeyer

Replace

if z != 0:

by

if z:

comment:7 Changed 4 years ago by git

  • Commit changed from cfc69b209bef0e1db4c4238c19db68c3012684f8 to cb3f891f66da3e2ecb13b6c08a0bfc40afff4882

Branch pushed to git repo; I updated commit sha1. New commits:

cb3f891adds default parameter to solve(). minor formatting changes.

comment:8 Changed 4 years ago by tgaona

  • Status changed from needs_work to needs_review

comment:9 Changed 4 years ago by jdemeyer

  • Branch changed from u/tgaona/ticket/19533 to u/jdemeyer/ticket/19533

comment:10 Changed 4 years ago by jdemeyer

  • 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:

c3ef1edFurther fixes to quadratic form solver

comment:11 Changed 4 years ago by tgaona

  • Status changed from needs_review to positive_review

comment:12 Changed 4 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/19533 to c3ef1ed9ae3cfece81f4587b3f824e8ba115f2ec
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.