## #8722 closed defect (fixed)

# S-units sometimes broken and sometimes just plain wrong for relative fields

Reported by: | David Loeffler | Owned by: | David Loeffler |
---|---|---|---|

Priority: | major | Milestone: | sage-4.4 |

Component: | number fields | Keywords: | |

Cc: | Merged in: | sage-4.4.alpha2 | |

Authors: | David Loeffler | Reviewers: | John Cremona |

Report Upstream: | N/A | Work issues: | |

Branch: | Commit: | ||

Dependencies: | Stopgaps: |

### Description (last modified by )

The code for S-unit groups of number fields calls the `degree`

method. For relative number fields this deliberately returns an error, because of the ambiguity between absolute and relative degree.

sage: L.<a,b> = NumberField([x^2 + 1, x^2 - 5]) sage: sage: p = L.ideal((-1/2*b - 1/2)*a + 1/2*b - 1/2) sage: sage: W = L.S_units([p]); W --------------------------------------------------------------------------- NotImplementedError Traceback (most recent call last) ... NotImplementedError: For a relative number field you must use relative_degree or absolute_degree as appropriate

In this case I think it should be absolute_degree, but changing this returns wrong output:

sage: L.<a,b> = NumberField([x^2 + 1, x^2 - 5]) sage: p = L.ideal((-1/2*b - 1/2)*a + 1/2*b - 1/2) sage: p.absolute_norm() 9 sage: p.is_prime() True sage: W = L.S_units([p]); W [1/2*a + 7/4, a, 1/2*b - 1/2] sage: W[0].valuation(L.primes_above(2)[0]) -4

So the first element of the list of S-units isn't actually an S-unit!

In other examples the code just blows up, because it calls `residue_field`

and that dies because of #8721:

sage: L.<a, b> = NumberField([polygen(QQ)^2 - 3, polygen(QQ)^2 - 5]) sage: L.S_units([L.ideal(a)])

This is arguably less bad: raising an error is far better than silently a wrong answer.

### Attachments (1)

### Change History (6)

### comment:1 Changed 13 years ago by

Description: | modified (diff) |
---|

### comment:2 Changed 13 years ago by

Status: | new → needs_review |
---|

### comment:3 Changed 13 years ago by

Status: | needs_review → positive_review |
---|

Looks good, applied fine to 4.4.alpha0 + #8446 patches, and all tests in sage/rings/number_field pass.

Positive review!

### comment:4 Changed 13 years ago by

Authors: | → David Loeffler |
---|---|

Merged in: | → sage-4.4.alpha2 |

Resolution: | → fixed |

Reviewers: | → John Cremona |

Status: | positive_review → closed |

Merged into 4.4.alpha2.

### comment:5 Changed 13 years ago by

Milestone: | sage-5.0 → sage-4.4 |
---|

**Note:**See TracTickets for help on using tickets.

Here's a patch. Turns out that the code was using

`K.gen`

and the correct answer is to call`K.absolute_generator`

, which isn't the same in the above example. This fixes the first example; the second is an instance of #8721.