Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a method to find a vector x such that Q(x) = C, where Q is a quadratic form and C is a constant. #19533

Closed
sagetrac-tgaona mannequin opened this issue Nov 6, 2015 · 14 comments

Comments

@sagetrac-tgaona
Copy link
Mannequin

sagetrac-tgaona mannequin commented Nov 6, 2015

This algorithm can be implemented using PARI's qfsolve(). Will be used as a component for the method implemented in Ticket [ticket:19112]

CC: @annahaensch

Component: quadratic forms

Author: Tyler Gaona

Branch/Commit: c3ef1ed

Reviewer: Jeroen Demeyer

Issue created by migration from https://trac.sagemath.org/ticket/19533

@sagetrac-tgaona sagetrac-tgaona mannequin added this to the sage-6.10 milestone Nov 6, 2015
@sagetrac-tgaona sagetrac-tgaona mannequin self-assigned this Nov 6, 2015
@sagetrac-tgaona
Copy link
Mannequin Author

sagetrac-tgaona mannequin commented Nov 14, 2015

Branch: u/tgaona/ticket/19533

@sagetrac-tgaona
Copy link
Mannequin Author

sagetrac-tgaona mannequin commented Nov 14, 2015

New commits:

cfc69b2Adds the `solve` method for quadratic forms.

@sagetrac-tgaona
Copy link
Mannequin Author

sagetrac-tgaona mannequin commented Nov 14, 2015

Commit: cfc69b2

@jdemeyer
Copy link

comment:3

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.

@jdemeyer
Copy link

comment:4

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.

@jdemeyer
Copy link

comment:5

Another minor thing: I would change the error message to

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

@jdemeyer
Copy link

comment:6

Replace

if z != 0:

by

if z:

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 17, 2015

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 17, 2015

Changed commit from cfc69b2 to cb3f891

@jdemeyer
Copy link

Changed branch from u/tgaona/ticket/19533 to u/jdemeyer/ticket/19533

@jdemeyer
Copy link

Changed commit from cb3f891 to c3ef1ed

@jdemeyer
Copy link

comment:10

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

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@vbraun
Copy link
Member

vbraun commented Nov 19, 2015

Changed branch from u/jdemeyer/ticket/19533 to c3ef1ed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants