Opened 3 years ago

Closed 21 months ago

Last modified 21 months ago

#27749 closed defect (fixed)

is_globally_equivalent_to is not symmetric

Reported by: sbrandhorst Owned by:
Priority: major Milestone: sage-9.2
Component: quadratic forms Keywords: pari
Cc: slelievre, mjo, gh-kliem Merged in:
Authors: Simon Brandhorst Reviewers: Samuel Lelièvre
Report Upstream: Fixed upstream, in a later stable release. Work issues:
Branch: be9f880 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by slelievre)

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

Change History (16)

comment:1 Changed 3 years ago by sbrandhorst

  • Report Upstream changed from Reported upstream. No feedback yet. to Reported upstream. Developers acknowledge bug.

comment:2 Changed 3 years ago by slelievre

  • Cc slelievre added
  • Description modified (diff)
  • Keywords pari added
  • Report Upstream changed from Reported upstream. Developers acknowledge bug. to Fixed upstream, but not in a stable release.

comment:3 Changed 3 years ago by embray

  • Milestone sage-8.8 deleted

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).

comment:4 Changed 2 years ago by chapoton

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.

comment:5 Changed 2 years ago by mkoeppe

  • Cc mjo added

comment:6 Changed 2 years ago by mkoeppe

  • Cc gh-kliem added

comment:7 Changed 2 years ago by mkoeppe

See also discussion in #29472

comment:8 Changed 22 months ago by sbrandhorst

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)

comment:9 Changed 22 months ago by slelievre

The pari upgrade happened in

  • #29313: Upgrade to pari 2.11.4

which was merged in Sage 9.2.beta7.

comment:10 Changed 22 months ago by slelievre

  • Milestone set to sage-9.2
  • Report Upstream changed from Fixed upstream, but not in a stable release. to Fixed upstream, in a later stable release.

Can someone add the doctest?

comment:11 Changed 22 months ago by sbrandhorst

  • Branch set to u/sbrandhorst/is_globally_equivalent_to_is_not_symmetric

comment:12 Changed 22 months ago by sbrandhorst

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

New commits:

be9f880document that the bug in equivalence testing is fixed

comment:13 Changed 22 months ago by slelievre

  • Authors set to Simon Brandhorst
  • Reviewers set to Samuel Lelièvre
  • Status changed from needs_review to positive_review

comment:14 Changed 21 months ago by vbraun

  • Branch changed from u/sbrandhorst/is_globally_equivalent_to_is_not_symmetric to be9f8807a8e416dc117e010872641da56d2c9a88
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:15 follow-up: Changed 21 months ago by dimpase

  • Commit be9f8807a8e416dc117e010872641da56d2c9a88 deleted

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?

comment:16 in reply to: ↑ 15 Changed 21 months ago by dimpase

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

Note: See TracTickets for help on using tickets.