Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#23376 closed defect (fixed)

Equality testing of genera of quadratic forms over ZZ changes the genus and produces false results

Reported by: sbrandhorst Owned by:
Priority: major Milestone: sage-8.0
Component: quadratic forms Keywords: sd87, Genus, sd91
Cc: Merged in:
Authors: Simon Brandhorst Reviewers: Anthony Várilly-Alvarado, Jen Berg
Report Upstream: N/A Work issues:
Branch: 24a5b0b (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by vdelecroix) 

sage: D4 = QuadraticForm(Matrix(ZZ,4,4,[2,0,0,-1,0,2,0,-1,0,0,2,-1,-1,-1,-1,2]))
sage: G = D4.global_genus_symbol()
sage: sage.quadratic_forms.genera.genus.is_GlobalGenus(G)
True
sage: G == copy(G)
True
sage: sage.quadratic_forms.genera.genus.is_GlobalGenus(G)
False

The reason is that various functions unintentionally modify their input. Since a genus consists of lists containing lists, functions do just copy the lists but not deepcopy.

For example sage.quadratic_forms.genera.genus.canonical_2_adic_reduction modifies its input which does not seem to be intended.

Change History (16)

comment:1 Changed 4 years ago by sbrandhorst

  • Branch set to u/sbrandhorst/equality_testing_of_genera_of_quadratic_forms_over_zz_changes_the_genus_and_produces_false_results

comment:2 Changed 4 years ago by sbrandhorst

  • Commit set to 88bdf3c89c5522df62d0035cd3d8826755d451d2
  • Status changed from new to needs_review

New commits:

​88bdf3cFixed two bugs in the genus class

comment:3 Changed 4 years ago by sbrandhorst

  • Status changed from needs_review to needs_work

comment:4 Changed 4 years ago by git

  • Commit changed from 88bdf3c89c5522df62d0035cd3d8826755d451d2 to 8e62210714bf1bcc426733eab9848bd3ff230c23

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

​8e62210Seems that is_2_adic_genus did not assume the input to be canonical but instead did not reduce the determinants mod 8. Changed that.

comment:5 Changed 4 years ago by sbrandhorst

  • Status changed from needs_work to needs_review

comment:6 Changed 4 years ago by vdelecroix

  • Description modified (diff)

(reformatting)

comment:7 Changed 4 years ago by sbrandhorst

  • Keywords sd87 added

comment:8 Changed 4 years ago by sbrandhorst

  • Keywords sd91 added; sd87 removed

comment:9 Changed 3 years ago by edgarcosta

  • Keywords sd87 added

You should leave the sd87 keyword there.

comment:10 Changed 3 years ago by jensberg

  • Status changed from needs_review to needs_work

Doesn't follow the doctest conventions; needs empty line after TESTS::

comment:11 Changed 3 years ago by git

  • Commit changed from 8e62210714bf1bcc426733eab9848bd3ff230c23 to 24a5b0bcc3f31e1dcf08ce88c0c709578a8bbf7c

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

​a7cf87eMerge branch 'u/bhutz/projective_infinity' of git://trac.sagemath.org/sage into t/23376/equality_testing_of_genera_of_quadratic_forms_over_zz_changes_the_genus_and_produces_false_results
​24a5b0bDoctest formatting.

comment:12 Changed 3 years ago by avarilly

  • Status changed from needs_work to positive_review

Fix looks good now. Passes all doc tests and documentation builds.

comment:13 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

Reviewer name is missing...

comment:14 Changed 3 years ago by jensberg

  • Reviewers set to Anthony Varilly-Alvarado, Jen Berg
  • Status changed from needs_work to positive_review

Added reviewer names.

comment:15 Changed 3 years ago by vbraun

  • Branch changed from u/sbrandhorst/equality_testing_of_genera_of_quadratic_forms_over_zz_changes_the_genus_and_produces_false_results to 24a5b0bcc3f31e1dcf08ce88c0c709578a8bbf7c
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:16 Changed 3 years ago by jdemeyer

  • Commit 24a5b0bcc3f31e1dcf08ce88c0c709578a8bbf7c deleted
  • Reviewers changed from Anthony Varilly-Alvarado, Jen Berg to Anthony Várilly-Alvarado, Jen Berg
Note: See TracTickets for help on using tickets.