Opened 13 years ago

Closed 13 years ago

#8124 closed enhancement (fixed)

Selmer groups for number fields

Reported by: Robert Miller Owned by: David Loeffler
Priority: major Milestone: sage-4.3.3
Component: number fields Keywords:
Cc: Merged in: sage-4.3.3.alpha0
Authors: Robert Miller Reviewers: John Cremona, Christian Wuthrich
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

I forgot to include this function in my big S-units and S-class groups patch.

I've tested on my laptop and on sage.math, so I think this passes on 32 and 64 bit systems.

Attachments (1)

trac_8124-selmer-nf.review.patch (4.0 KB) - added by Robert Miller 13 years ago.
Replaces previous (fixes latex glitch)

Download all attachments as: .zip

Change History (10)

comment:1 Changed 13 years ago by Robert Miller

Status: newneeds_review

comment:2 Changed 13 years ago by wuthrich

Would it be much more work to make this function to give back a finite abelian group ? In the long run, we should make sure that these sort of functions return groups. It may be better to try get the correct type from the beginning as not to break later things that use it.

(Sorry to be criticising this again. I think you are doing a great job implementing all these things.)

comment:3 Changed 13 years ago by John Cremona

Reviewers: cremona, wuthrich
Status: needs_reviewpositive_review

Looks good; tests pass. Building the docs (reference html) revealed a problem in polynomial_quotient_ring.py, preexisting in the selmer_group function: the macro "\cross" was not recognised. I could not get "\times" to work instead (which is what I normally use for this) but * does, so I put that in.

Regarding Chris's comment: of course I agree, this should be a proper abelian group. But even making it a semi-functional abelian group of the sort used for unit groups and class groups would require quite a bit of extra work, since this code finds generators but _not_ the group structure. So I think that should be in another ticket. (For example, I would like to use this function for m=4 and will try to do so. In that case there will in general be generators of order 2 as well as some of order 4.)

comment:4 Changed 13 years ago by wuthrich

Ok, I see and I also understand that this less costly function should be available even if we had selmer_groups given as abelian groups. Maybe a renaming to selmer_group_gens() would be adequate then we can use it later and add later selmer_group without having to change the type return by a function.

But I won't oppose the positive review on this ticket, of course.

comment:5 Changed 13 years ago by Robert Miller

Reviewers: cremona, wuthrichJohn Cremona, Christian Wuthrich

It looks like someone has already fixed the issue John's patch here fixes, so the release manager should merge the original patch.

If there are no objections, I will remove the second patch.

comment:6 in reply to:  3 Changed 13 years ago by Robert Miller

Replying to cremona:

I could not get "\times" to work instead (which is what I normally use for this) but * does, so I put that in.

Actually, it looks like someone changed this to \times in rc0, so I've updated the patch to change \times instead of \cross to *.

Changed 13 years ago by Robert Miller

Replaces previous (fixes latex glitch)

comment:7 Changed 13 years ago by John Cremona

Patch trac_8124-selmer-nf.review.patch applies fine to 4.3.2.alpha1 and tests pass and doc build ok!

The tag was already at review, and I'm happy to leave it there.

comment:8 in reply to:  7 Changed 13 years ago by John Cremona

Replying to cremona:

Patch trac_8124-selmer-nf.review.patch applies fine to 4.3.2.alpha1 and tests pass and doc build ok!

The tag was already at review, and I'm happy to leave it there.

That should have said "positive review".

I checked that this applies fine to 4.3.2 + #8184 spkg & patches + #8155 patches; all tests pass (on 64-bit).

comment:9 Changed 13 years ago by Mitesh Patel

Merged in: sage-4.3.3.alpha0
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.