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

is_globally_equivalent_to is not symmetric #27749

Closed
simonbrandhorst opened this issue Apr 29, 2019 · 20 comments
Closed

is_globally_equivalent_to is not symmetric #27749

simonbrandhorst opened this issue Apr 29, 2019 · 20 comments

Comments

@simonbrandhorst
Copy link

Q = QuadraticForm(ZZ, 2, [2,3,5])
P = QuadraticForm(ZZ, 2, [8,6,5])
Q.is_globally_equivalent_to(P)
True
P.is_globally_equivalent_to(Q)
False

The expected output in any case is False.
The underlying computation is done with Pari.

Upstream ticket (PARI):

Upstream: Fixed upstream, in a later stable release.

CC: @slel @orlitzky @kliem

Component: quadratic forms

Keywords: pari

Author: Simon Brandhorst

Branch: be9f880

Reviewer: Samuel Lelièvre

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

@simonbrandhorst
Copy link
Author

Changed upstream from Reported upstream. No feedback yet. to Reported upstream. Developers acknowledge bug.

@slel
Copy link
Member

slel commented Apr 29, 2019

Changed upstream from Reported upstream. Developers acknowledge bug. to Fixed upstream, but not in a stable release.

@slel

This comment has been minimized.

@slel
Copy link
Member

slel commented Apr 29, 2019

Changed keywords from none to pari

@embray
Copy link
Contributor

embray commented Jun 14, 2019

comment:3

As the Sage-8.8 release milestone is pending, we should delete the sage-8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage-8.9).

@embray embray removed this from the sage-8.8 milestone Jun 14, 2019
@fchapoton
Copy link
Contributor

comment:4

This is still failing in pari 2.11.2 from May 2019 (currently in sage)

but works in pari [2, 11, 4, 23175, "e4f812ce9f"].

This will need a doctest once pari is updated.

@mkoeppe
Copy link
Member

mkoeppe commented Apr 28, 2020

comment:7

See also discussion in #29472

@simonbrandhorst
Copy link
Author

comment:8
sage: Q = QuadraticForm(ZZ, 2, [2,3,5]) 
....: P = QuadraticForm(ZZ, 2, [8,6,5]) 
....: Q.is_globally_equivalent_to(P)                                         
False
sage: P.is_globally_equivalent_to(Q)                                         
False
sage: pari.version()                                                         
(2, 11, 4)

@slel
Copy link
Member

slel commented Sep 26, 2020

comment:9

The pari upgrade happened in

which was merged in Sage 9.2.beta7.

@slel
Copy link
Member

slel commented Sep 26, 2020

Changed upstream from Fixed upstream, but not in a stable release. to Fixed upstream, in a later stable release.

@slel
Copy link
Member

slel commented Sep 26, 2020

comment:10

Can someone add the doctest?

@slel slel added this to the sage-9.2 milestone Sep 26, 2020
@simonbrandhorst
Copy link
Author

@simonbrandhorst
Copy link
Author

Commit: be9f880

@simonbrandhorst
Copy link
Author

New commits:

be9f880document that the bug in equivalence testing is fixed

@slel
Copy link
Member

slel commented Sep 27, 2020

Reviewer: Samuel Lelièvre

@slel
Copy link
Member

slel commented Sep 27, 2020

Author: Simon Brandhorst

@vbraun
Copy link
Member

vbraun commented Oct 3, 2020

@dimpase
Copy link
Member

dimpase commented Oct 20, 2020

comment:15

Shouldn't a test be added to spkg-configure.m4 of pari? Note that e.g. on Fedora 30 one has pari/gp version 2.11.2, which passes all the tests we currently have there, but fails the test in
quadratic_form__equivalence_testing added by this ticket?

@dimpase
Copy link
Member

dimpase commented Oct 20, 2020

Changed commit from be9f880 to none

@dimpase
Copy link
Member

dimpase commented Oct 20, 2020

comment:16

Replying to @dimpase:

Shouldn't a test be added to spkg-configure.m4 of pari? Note that e.g. on Fedora 30 one has pari/gp version 2.11.2, which passes all the tests we currently have there, but fails the test in
quadratic_form__equivalence_testing added by this ticket?

should be fixed by #30800

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

7 participants