Opened 6 years ago

Closed 4 months ago

#18863 closed defect (fixed)

Subgroup doesn't work with number field unit group

Reported by: katestange Owned by:
Priority: major Milestone: sage-9.4
Component: group theory Keywords: unit group, number field, subgroup, gap
Cc: katestange, tscholl2 Merged in:
Authors: Travis Scholl Reviewers: David Roe
Report Upstream: N/A Work issues:
Branch: 2832310 (Commits, GitHub, GitLab) Commit: 2832310d0304806c21cd9b08ca5f0d98f135c743
Dependencies: Stopgaps:

Status badges

Description (last modified by katestange)

I think we would like the following code to work:

N.<a> = NumberField(x^3+2)
G = N.unit_group()
g = G.random_element()
G.subgroup([g])

But at the moment the last line produces a runtime error

RuntimeError: Gap produced error output
Error, Variable: 'u1' must have a value

This is currently reproducible on the single cell sage server and sage cloud. It does not seem to depend on the number field in question.

As far as I can tell, subgroup() is unable to recognise the input as elements of the group. I think the problem is that one cannot pass the argument 'names' when creating the unit group. For example, the following works:

H = AbelianGroup(5,[2],names=list("pqrst"))
H.subgroup([H.random_element()])

but the following fails in exactly the same way as the unit group example

H = AbelianGroup(5,[2])
H.subgroup([H.random_element()])

Change History (16)

comment:1 Changed 6 years ago by katestange

  • Priority changed from minor to major

comment:2 Changed 6 years ago by katestange

  • Description modified (diff)

comment:3 Changed 6 years ago by katestange

  • Description modified (diff)

comment:4 Changed 6 years ago by katestange

  • Description modified (diff)

comment:5 Changed 6 years ago by katestange

I asked for a workaround here, and there is some more information from other users there: http://ask.sagemath.org/question/27274/subgroup-of-number-field-unit-group/

comment:6 Changed 6 years ago by katestange

  • Keywords gap added

comment:7 Changed 6 years ago by fwclarke

I note that the same problem arises with class groups, which are also built using AbelianGroupWithValues. However in the following case, it is possible to obtain a subgroup of a group with values:

sage: U = Zmod(30).unit_group()
sage: type(U)
<class 'sage.groups.abelian_gps.values.AbelianGroupWithValues_class_with_category'>
sage: U.subgroup([U.random_element()])
Multiplicative Abelian subgroup isomorphic to C2 generated by {f0*f1^2}

I have tried, but failed, to see why the other cases lead to gap errors.

comment:8 Changed 16 months ago by tscholl2

I believe this is another small example, and it raises an exception in at least Sage 9.0.

G = AbelianGroup([4,0])
G.subgroup(G.gens())

I printed out the gap commands which this runs in the gap.eval(...) lines:

G:= AbelianGroup([4]); gens := GeneratorsOfGroup(G)
f0 := gens[1]
gensH:=[f0, f1]

It seemed clear that the issue was likely the order check here:

https://github.com/sagemath/sage/blob/860e4dc9881966a36ef8808a0d1fae0c6b54f741/src/sage/groups/abelian_gps/abelian_group.py#L1549

Changing those lines to the following seem to fix it:

Hgens0 = [x for x in Hgens if any(y in str(x) for y in Ggens0)]
Hgensf = [x for x in Hgens if not(x in Hgens0)]

This seemed to also make the code in the original post work, i.e.:

H = AbelianGroup(5,[2])
H.subgroup([H.random_element()])

I have not tested this rigorously yet, and I have no idea if this is the "proper" way to test whether a generator has infinite order. I am also unsure if that is the thing which is being tested here.

comment:9 Changed 16 months ago by tscholl2

  • Authors set to Travis Scholl
  • Branch set to u/tscholl2/18863
  • Cc tscholl2 added
  • Commit set to 5eef20d721d67b86604715336f1436ce3fbe88bb
  • Status changed from new to needs_review

I modified the lines I mentioned in my previous comment, however I tried to avoid using string representations and instead looked at the order of each generator appearing in each element given as a generator for the subgroup.

It passed all tests for me when running ./sage -t src/sage/groups/* and ./sage -t --long src/sage/groups/abelian_gps/*.

comment:10 Changed 14 months ago by gh-mwageringel

  • Milestone changed from sage-6.8 to sage-9.2

comment:11 Changed 11 months ago by mkoeppe

  • Milestone changed from sage-9.2 to sage-9.3

comment:12 Changed 6 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.

comment:13 Changed 5 months ago by roed

  • Branch changed from u/tscholl2/18863 to u/roed/18863

comment:14 Changed 5 months ago by roed

  • Commit changed from 5eef20d721d67b86604715336f1436ce3fbe88bb to 2832310d0304806c21cd9b08ca5f0d98f135c743
  • Reviewers set to David Roe

I deleted a couple lines to make pyflakes happy; set to positive review once tests pass.


New commits:

ed9a2d3Merge branch 'u/tscholl2/18863' of git://trac.sagemath.org/sage into t/18863/subgroup
2832310Remove unused variable to make pyflakes happy

comment:15 Changed 5 months ago by roed

  • Status changed from needs_review to positive_review

comment:16 Changed 4 months ago by vbraun

  • Branch changed from u/roed/18863 to 2832310d0304806c21cd9b08ca5f0d98f135c743
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.