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

bugs for binary quadratic forms whose discriminant is a square #29028

Closed
DaveWitteMorris opened this issue Jan 16, 2020 · 17 comments
Closed

bugs for binary quadratic forms whose discriminant is a square #29028

DaveWitteMorris opened this issue Jan 16, 2020 · 17 comments

Comments

@DaveWitteMorris
Copy link
Member

Ticket #28989 fixes bugs for quadratic forms whose discriminant is positive, but not a square. This ticket addresses the remaining case where the discriminant is a square. Symptoms of the bugs include:

Two forms may be properly equivalent, but not equivalent, and equivalence is not reflexive:

sage: Q = BinaryQF(0, 2, 0)
sage: Q.discriminant()
4
sage: Q.is_equivalent(Q, proper=True)
True
sage: Q.is_equivalent(Q, proper=False)
False

The forms -4*x^2 + 10*x*y, -2*x^2 + 10*x*y, 10*x*y, 2*x^2 + 10*x*y, 4*x^2 + 10*x*y, and 5*x^2 + 10*x*y are not primitive (i.e., the gcd of the coefficients is not 1), but they appear in a list of primitive forms:

sage: BinaryQF_reduced_representatives(10^2, primitive_only=True)
[-4*x^2 + 10*x*y,
 -3*x^2 + 10*x*y,
 -2*x^2 + 10*x*y,
 -x^2 + 10*x*y,
 10*x*y,
 x^2 + 10*x*y,
 2*x^2 + 10*x*y,
 3*x^2 + 10*x*y,
 4*x^2 + 10*x*y,
 5*x^2 + 10*x*y]

I will upload a PR soon.

Depends on #28989

CC: @simonbrandhorst

Component: number theory

Keywords: binary quadratic form

Author: Dave Morris

Branch/Commit: 4580620

Reviewer: Travis Scrimshaw

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

@DaveWitteMorris
Copy link
Member Author

Branch: public/29028

@DaveWitteMorris
Copy link
Member Author

New commits:

aa21ee1proper cycle should not have odd length
c24e07dapply tau only to half of the terms
6ac626arevise docstrings and fix another bug
2c2bc7adiscriminants that are a square

@DaveWitteMorris
Copy link
Member Author

Commit: 2c2bc7a

@DaveWitteMorris
Copy link
Member Author

Author: Dave Morris

@DaveWitteMorris
Copy link
Member Author

Dependencies: #28989

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 17, 2020

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

48d6e89docs should not have # comments
17653f7Merge latest #28989 into #29028
028d95bdocs should not have # comments

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 17, 2020

Changed commit from 2c2bc7a to 028d95b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 22, 2020

Changed commit from 028d95b to f2f6bd5

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 22, 2020

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

ee54527fix minor issues
f2f6bd5fix merge conflict with #28989

@DaveWitteMorris
Copy link
Member Author

comment:6

Merged in #28989 again, to fix a merge conflict caused by the latest changes there.

@tscrim
Copy link
Collaborator

tscrim commented Jan 29, 2020

comment:7

Two little changes:

         We check that the first part of :trac:`29028` is fixed::
+
             sage: Q = BinaryQF(0, 2, 0)
             sage: Q.discriminant()
             4
     Test another part of :trac:`29028`::
+
         sage: BinaryQF_reduced_representatives(10^2, proper=False, primitive_only=False)

as you need a blankline after the :: line.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2020

Changed commit from f2f6bd5 to 4580620

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2020

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

4580620need blank line after comment

@DaveWitteMorris
Copy link
Member Author

comment:9

Thanks for the corrections.

@tscrim
Copy link
Collaborator

tscrim commented Jan 29, 2020

comment:10

Thanks. LGTM.

@tscrim
Copy link
Collaborator

tscrim commented Jan 29, 2020

Reviewer: Travis Scrimshaw

@vbraun
Copy link
Member

vbraun commented Jan 31, 2020

Changed branch from public/29028 to 4580620

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

3 participants