Opened 9 years ago
Closed 8 years ago
#11304 closed defect (fixed)
Problems with S-class groups of number fields
Reported by: | fwclarke | Owned by: | davidloeffler |
---|---|---|---|
Priority: | major | Milestone: | sage-4.7.2 |
Component: | number fields | Keywords: | S-class groups |
Cc: | jdemeyer, cremona, rlm | Merged in: | sage-4.7.2.alpha1 |
Authors: | Francis Clarke | Reviewers: | John Cremona |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
There are some serious problems at present with the code for S-class groups. They only emerge when the class groups is non-cyclic. For example,
sage: K.<a> = QuadraticField(-105) sage: C = K.class_group(); C Class group of order 8 with structure C2 x C2 x C2 of Number Field in a with defining polynomial x^2 + 105 sage: S = (K.ideal(11, a + 7),) sage: K.S_class_group(S) Traceback (most recent call last): ... IndexError: Argument length (= 3) must be 2.
This problem arises when the class group and the S-class group have differing numbers of generators. It arises essentially because generators of S-class groups are created as FractionalIdealClass
elements rather than SFractionalIdealClass
elements.
But there is a more serious problem. The Pari data for the S-class group which we failed to construct above can be obtained as
sage: SC_data = K._S_class_group_and_units(S)[1]; SC_data [(Fractional ideal (10, a + 5), 2, 10), (Fractional ideal (6, a + 3), 2, 6)]
so that if
sage: P, Q = [u[0] for u in SC_data]
the S-classes of the ideals P
and Q
(each of order 2) generate the S-class group. However,
sage: P._S_ideal_class_log(S) [0, 0]
which cannot be correct.
Attachments (1)
Change History (8)
Changed 8 years ago by
comment:1 Changed 8 years ago by
- Cc jdemeyer cremona added
- Status changed from new to needs_review
The patch rectifies the defects. It also incorporates enhancements for both class groups and S-class groups.
In particular:
The computation of S-class groups is made faster when the ideals in S are all principal.
The computation of ideal_class_logs is speeded up by setting a pari flag.
The code for S-class groups is made more compatible with that for class groups.
comment:2 follow-up: ↓ 3 Changed 8 years ago by
- Cc rlm added
Looks good to me, and applies fine to 4.7.1.alpha1 and all tests pass.
I've added rlm to the CC list since I think he implemented this originally, and hope he'll take a look too.
comment:3 in reply to: ↑ 2 ; follow-up: ↓ 6 Changed 8 years ago by
Replying to cremona:
Looks good to me, and applies fine to 4.7.1.alpha1 and all tests pass.
I've added rlm to the CC list since I think he implemented this originally, and hope he'll take a look too.
As Robert doesn't seem to reply, do you want to give this a positive review, or have someone else review it too?
comment:4 Changed 8 years ago by
- Reviewers set to John Cremona
- Status changed from needs_review to positive_review
Let's give it a positive review, since I clearly thought it deserved that 5 weeks ago! Marco, you can add your name as reviewer if appropriate.
comment:5 Changed 8 years ago by
- Milestone changed from sage-4.7.1 to sage-4.7.2
comment:6 in reply to: ↑ 3 Changed 8 years ago by
Replying to mstreng:
Replying to cremona:
Looks good to me, and applies fine to 4.7.1.alpha1 and all tests pass.
I've added rlm to the CC list since I think he implemented this originally, and hope he'll take a look too.
As Robert doesn't seem to reply, do you want to give this a positive review, or have someone else review it too?
Sorry for the delay, but I can add that the changes here look good to me too.
comment:7 Changed 8 years ago by
- Merged in set to sage-4.7.2.alpha1
- Resolution set to fixed
- Status changed from positive_review to closed
apply after patches from #11234