Opened 13 years ago

Closed 13 years ago

# Selmer groups for number fields

Reported by: Owned by: Robert Miller David Loeffler major sage-4.3.3 number fields sage-4.3.3.alpha0 Robert Miller John Cremona, Christian Wuthrich N/A

### 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.

### comment:1 Changed 13 years ago by Robert Miller

Status: new → needs_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 follow-up:  6 Changed 13 years ago by John Cremona

Reviewers: → cremona, wuthrich needs_review → positive_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, wuthrich → John 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

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 follow-up:  8 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.