Opened 3 years ago
Closed 22 months ago
#29364 closed defect (fixed)
Bug in Sclass groups of number fields
Reported by:  Benjamin Matschke  Owned by:  

Priority:  major  Milestone:  sage9.3 
Component:  number fields  Keywords:  S_class_group, fractional_ideal 
Cc:  Merged in:  
Authors:  Dave Morris  Reviewers:  Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  677111e (Commits, GitHub, GitLab)  Commit:  677111e60524cc2e8a37deb31d766a72182bdefe 
Dependencies:  Stopgaps: 
Description
Coercing a fractional ideal of a number field to an Sclass group over the same number field sometimes raises a KeyError? and an AssertionError?.
Here is a failing example:
R.<x> = QQ[] L.<t> = NumberField(x^2  6058) S = L.primes_above(2) C = L.S_class_group(S) J = L.fractional_ideal(1) print("C(J):", C(J)) # Should print the trivial Sideal class of C, but raises errors.
This was tested in Sage 9.0, 8.8, and 8.6, always yielding the same error.
Change History (11)
comment:1 Changed 2 years ago by
Milestone:  sage9.1 → sage9.2 

comment:2 Changed 2 years ago by
Milestone:  sage9.2 → sage9.3 

comment:3 Changed 22 months ago by
Branch:  → public/29364 

comment:4 Changed 22 months ago by
Authors:  Benjamin Matschke → Dave Morris 

Commit:  → 0241a7422f63e8c6900e37db3f6f10c0da995a05 
Status:  new → needs_review 
The bug is in the NumberField._S_class_group_quotient_matrix()
method.
sage: R.<x> = QQ[] ....: L.<t> = NumberField(x^2  6058) ....: S = L.primes_above(2) ....: L._S_class_group_quotient_matrix(tuple(S))  KeyError Traceback (most recent call last) ... During handling of the above exception, another exception occurred: AssertionError Traceback (most recent call last) ... /Applications/math/sage9.2/local/lib/python3.7/sitepackages/sage/rings/number_field/number_field.py in _S_class_group_quotient_matrix(self, S) 4644 M = matrix(ZZ, M) 4645 A, Q = M.hermite_form(transformation=True) > 4646 assert A[:c] == 1 and A[c:] == 0 4647 return Q[:c, :a] 4648 AssertionError:
The rows of the matrix M are generators of <S> and generators of the quotient I/<S>, where I is the ideal class group, so the rows generate I. The algorithm assumed that lifting these generators to Z^{n} would provide a set of generators of Z^{n}, but that is not usually true.
The PR adds additional rows to M that are generators of the kernel of the homomorphism from Z^{n} to I. (Each generator of the kernel has only one nonzero entry, which is the order of the corresponding generator of I.) This squashes the bug:
sage: R.<x> = QQ[] ....: L.<t> = NumberField(x^2  6058) ....: S = L.primes_above(2) ....: C = L.S_class_group(S) ....: J = L.fractional_ideal(1) ....: print("C(J):", C(J)) C(J): Trivial Sideal class
(The fix is only one line of code, but the PR also adds a doctest, and deletes some spaceonly lines elsewhere in the file.)
New commits:
0241a74  fix trac 29364 Sclass group

comment:5 Changed 22 months ago by
doctest failure in src/sage/rings/number_field/number_field.py, see patchbot
comment:7 Changed 22 months ago by
Commit:  0241a7422f63e8c6900e37db3f6f10c0da995a05 → 677111e60524cc2e8a37deb31d766a72182bdefe 

Branch pushed to git repo; I updated commit sha1. New commits:
677111e  reviewer corrections

comment:8 Changed 22 months ago by
Oops! Thanks for pointing out that I goofed up the trac link.
I was surprised that the patchbot failed the doctest that I wrote. (It works fine on my computer.) But the matrix is not unique, so maybe it's more surprising that the other doctests of this method don't fail. Anyway, I wrote a new doctest that checks whether the matrix is valid, rather than requiring a certain value. I hope the patchbots will be happy with this one.
comment:9 Changed 22 months ago by
Reviewers:  → Frédéric Chapoton 

Status:  needs_review → positive_review 
ok, let it be (not a mathematical review, though)
comment:11 Changed 22 months ago by
Branch:  public/29364 → 677111e60524cc2e8a37deb31d766a72182bdefe 

Resolution:  → fixed 
Status:  positive_review → closed 
Moving tickets to milestone sage9.2 based on a review of last modification date, branch status, and severity.