Opened 8 years ago
Closed 8 years ago
#16379 closed enhancement (fixed)
Compute Hasse invariant over number fields and fix current implementation
Reported by:  annahaensch  Owned by:  

Priority:  minor  Milestone:  sage6.3 
Component:  quadratic forms  Keywords:  
Cc:  Merged in:  
Authors:  Anna Haensch  Reviewers:  Peter Bruin 
Report Upstream:  N/A  Work issues:  
Branch:  f8fec47 (Commits, GitHub, GitLab)  Commit:  f8fec470e7514f2ad4d8400da4006f63a1d9a2ff 
Dependencies:  Stopgaps: 
Description
This patch will enhance the current functions hasse_invariant and hasse_invariant_OMeara to include computations of Hasse invariant for quadratic forms over number fields. In addition, this patch fixes an indexing error in the original hasse_invariant_OMeara code, which currently reads
for j in range(n1): for k in range(j, n):
but should read
for j in range(n): for k in range(j, n):
to be consistent with OMeara's algorithm.
Change History (10)
comment:1 Changed 8 years ago by
 Branch set to u/annahaensch/ticket/16379
 Created changed from 05/20/14 09:25:46 to 05/20/14 09:25:46
 Modified changed from 05/20/14 09:25:46 to 05/20/14 09:25:46
comment:2 Changed 8 years ago by
 Commit set to 471f15734adb60b20b0bc3d6c37aeaa5c2c2e5f5
comment:3 Changed 8 years ago by
 Status changed from new to needs_review
comment:4 followup: ↓ 6 Changed 8 years ago by
 Reviewers set to Peter Bruin
Looks good to me except for two minor points:
 it is best to keep existing doctests as far as possible; only add new ones, and fix old ones if necessary instead of replacing them;
 your patch introduces trailing whitespace on several lines (use e.g.
git diff color develop...YOURBRANCH
to see this); this is discouraged, so could you remove it if and when you make another commit?
(The patchbot encountered a doctest failure on 6.3.beta1, but it seems to be unrelated to your patch.)
comment:5 Changed 8 years ago by
 Commit changed from 471f15734adb60b20b0bc3d6c37aeaa5c2c2e5f5 to a6fc438f5ec644581e6ba962bf9cab1a4a4a6253
Branch pushed to git repo; I updated commit sha1. New commits:
a6fc438  Removed trailing whitespace from last commit

comment:6 in reply to: ↑ 4 Changed 8 years ago by
Coming back to this point:
 it is best to keep existing doctests as far as possible; only add new ones, and fix old ones if necessary instead of replacing them;
I now see that there were two identical doctests (with DiagonalQuadraticForm(ZZ, [1, 1, 1])
) and you changed [1, 1, 1]
to [1, 1, 5]
in both of them. Could you change one of them back to [1, 1, 1]
? Then we get both your new doctest and the old one, instead of duplicates.
comment:7 Changed 8 years ago by
 Commit changed from a6fc438f5ec644581e6ba962bf9cab1a4a4a6253 to fb0d36017bc6118efe57935885464790b94c9d23
Branch pushed to git repo; I updated commit sha1. New commits:
fb0d360  Reverted one example in doctest

comment:8 Changed 8 years ago by
 Commit changed from fb0d36017bc6118efe57935885464790b94c9d23 to f8fec470e7514f2ad4d8400da4006f63a1d9a2ff
Branch pushed to git repo; I updated commit sha1. New commits:
f8fec47  Reverts one doctest to old example, ignore last commit

comment:9 Changed 8 years ago by
 Status changed from needs_review to positive_review
comment:10 Changed 8 years ago by
 Branch changed from u/annahaensch/ticket/16379 to f8fec470e7514f2ad4d8400da4006f63a1d9a2ff
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
updated examples from last commit