Opened 5 years ago
Closed 5 years ago
#25023 closed enhancement (fixed)
Adds function to compute quadratic defect
Reported by: | annahaensch | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.2 |
Component: | number fields | Keywords: | |
Cc: | Merged in: | ||
Authors: | Anna Haensch | Reviewers: | Simon Brandhorst |
Report Upstream: | N/A | Work issues: | |
Branch: | 56283fe (Commits, GitHub, GitLab) | Commit: | 56283fe849868db720531ad45364aa247050247a |
Dependencies: | Stopgaps: |
Description
Computes the quadratic defect (see O'Meara Section 63A) of a number field element locally at a prime p. This is an implementation of Algorithm 3.1.3 from Kirschmer's "Definite quadratic and hermitian forms with small class number."
The function returns the valuation of the quadratic defect (rather than the fractional ideal itself) since this is the more relevant (and computationally useful) piece of information.
Change History (15)
comment:1 Changed 5 years ago by
Branch: | → u/annahaensch/adds_function_to_compute_quadratic_defect |
---|
comment:2 Changed 5 years ago by
Commit: | → a140e55ed69a77b79bacdc0a1f68a316868f561d |
---|---|
Status: | new → needs_review |
comment:3 Changed 5 years ago by
Status: | needs_review → needs_work |
---|
Some quick comments:
over QQ
:
do not use quadratic_residues as that forms a list of size (p-1)/2 this is not necessary. You can use legendre_symbol or reduce mod p and then use is_square (like over numberfields)
Your method quadratic_defect sits in the wrong class. (NumberField_quadratic) so it is not accessible in general. For example we can move it to NumberField_generic.
+ if w < u and w % 2 ==1: + d = v+w + if w == u and (f+F((a-1)/4)).is_irreducible(): + d = v+w
You sure this is correct ?(I did not fully check).
- the reference to kirschmers work should be a proper citation in src/doc/en/reference/references
Could you explain why you consider the valuation more useful than the quadratic_defect itself?
comment:4 Changed 5 years ago by
Reviewers: | → Simon Brandhorst |
---|
comment:5 Changed 5 years ago by
Commit: | a140e55ed69a77b79bacdc0a1f68a316868f561d → e419b36841853392158db03d38ba6e006ea5d4d7 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
e419b36 | Changes to quadrati_defect()
|
comment:6 Changed 5 years ago by
Thanks for the helpful comments! I've done the following:
- Updated the function in rational_field to use legendre_symbol() instead of quadratic_residue().
- I added Kirschmer's item to the reference list, and added a reference line to the documentation. I've never done this before, perhaps up can see if I did it correctly?
- I moved the number field version of the function to be in the class number_field_generic...I think. Is this accomplished by just moving the function to where that class is defined? Maybe you can check to see if I did that correctly.
- The item you point out should be correct, this should be 63:3 and 63:5 from O'meara's book on quadratic forms.
I am choosing to return a valuation since this is an easier object to deal with. Moreover, this is also what is returned by the analogous function "QuadraticDefect?" in magma (which I believe was implimented by Kirschmer).
New commits:
e419b36 | Changes to quadrati_defect()
|
comment:7 Changed 5 years ago by
Status: | needs_work → needs_review |
---|
comment:8 Changed 5 years ago by
Branch: | u/annahaensch/adds_function_to_compute_quadratic_defect → u/sbrandhorst/adds_function_to_compute_quadratic_defect |
---|
comment:9 Changed 5 years ago by
Branch: | u/sbrandhorst/adds_function_to_compute_quadratic_defect → u/annahaensch/adds_function_to_compute_quadratic_defect |
---|
I did some cosmetic changes. Positive review if you are happy with what I have done.
comment:10 Changed 5 years ago by
Commit: | e419b36841853392158db03d38ba6e006ea5d4d7 → 086cde098120cf2313030c28ac33e2d80cc4a579 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
086cde0 | Add reference to rational_field
|
comment:11 Changed 5 years ago by
I just added the official reference to the rational_field version of the function. Now I am happy with it, and would be most pleased with a positive review. Thanks!
comment:12 Changed 5 years ago by
Status: | needs_review → positive_review |
---|
comment:13 Changed 5 years ago by
Branch: | u/annahaensch/adds_function_to_compute_quadratic_defect → u/sbrandhorst/adds_function_to_compute_quadratic_defect |
---|
comment:14 Changed 5 years ago by
Commit: | 086cde098120cf2313030c28ac33e2d80cc4a579 → 56283fe849868db720531ad45364aa247050247a |
---|
a trivial addition REFERENCES:: --> REFERENCES:
New commits:
e5f4670 | Allowed prime ideals as input for quadratic_defect.
|
b5fd719 | Merge branch 'u/annahaensch/adds_function_to_compute_quadratic_defect' of git://trac.sagemath.org/sage into t/25023/adds_function_to_compute_quadratic_defect
|
56283fe | Doc syntax
|
comment:15 Changed 5 years ago by
Branch: | u/sbrandhorst/adds_function_to_compute_quadratic_defect → 56283fe849868db720531ad45364aa247050247a |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
New commits:
adds imports to number_field.py
adds quadratic_defect