Opened 8 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)

trac_11304_S_class_groups.patch (16.7 KB) - added by fwclarke 8 years ago.
apply after patches from #11234

Download all attachments as: .zip

Change History (8)

Changed 8 years ago by fwclarke

apply after patches from #11234

comment:1 Changed 8 years ago by fwclarke

  • Authors set to Francis Clarke
  • 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: Changed 8 years ago by cremona

  • 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: Changed 8 years ago by 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?

comment:4 Changed 8 years ago by cremona

  • 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 jdemeyer

  • Milestone changed from sage-4.7.1 to sage-4.7.2

comment:6 in reply to: ↑ 3 Changed 8 years ago by rlm

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 jdemeyer

  • Merged in set to sage-4.7.2.alpha1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.