Skip to content

Commit 705cab1

Browse files
author
Release Manager
committed
gh-37226: unit group of number field: do not expand product when S is empty <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> This PR fixes #36386, at least for unit group computations (S-units will still suffer from the problem, or when the units are even larger). This PR does **not** address #31754, which would be a better (but more complicated) solution for the problem. <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes #12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies None <!-- List all open PRs that this PR logically depends on - #12345: short description why this is a dependency - #34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: #37226 Reported by: AurelPage Reviewer(s): AurelPage, Vincent Delecroix
2 parents 76b1a92 + 32c07c8 commit 705cab1

File tree

1 file changed

+19
-11
lines changed

1 file changed

+19
-11
lines changed

src/sage/rings/number_field/unit_group.py

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,14 @@ def __init__(self, number_field, proof=True, S=None):
305305
sage: tuple(US(K(u)) for u in US.gens()) == US.gens()
306306
True
307307
308+
Bug :issue:`36386` (pari stack overflow while expanding units)::
309+
310+
sage: d = 12936642
311+
sage: K = QuadraticField(d)
312+
sage: K.unit_group(proof=False)
313+
Unit group with structure C2 x Z of Number Field in a with defining polynomial x^2 - 12936642 with a = 3596.754370262167?
314+
315+
308316
"""
309317
proof = get_flag(proof, "number_field")
310318
K = number_field
@@ -340,20 +348,20 @@ def __init__(self, number_field, proof=True, S=None):
340348
# compute the additional S-unit generators:
341349
if S:
342350
self.__S_unit_data = pK.bnfunits(pS)
351+
# TODO: converting the factored matrix representation of bnfunits into polynomial
352+
# form is a *big* waste of time
353+
su = [pK.nfbasistoalg(pK.nffactorback(z)) for z in self.__S_unit_data[0][0:len(S)]]
354+
su = [K(u, check=False) for u in su]
343355
else:
344-
self.__S_unit_data = pK.bnfunits()
345-
# TODO: converting the factored matrix representation of bnfunits into polynomial
346-
# form is a *big* waste of time
347-
su_fu_tu = [pK.nfbasistoalg(pK.nffactorback(z)) for z in self.__S_unit_data[0]]
348-
349-
self.__nfu = len(pK.bnf_get_fu()) # number of fundamental units
350-
self.__nsu = len(su_fu_tu) - self.__nfu - 1 # number of S-units
351-
self.__ntu = pK.bnf_get_tu()[0] # order of torsion
356+
su = []
357+
358+
self.__nfu = len(fu) # number of fundamental units
359+
self.__nsu = len(su) # number of S-units
360+
self.__ntu = pK.bnf_get_tu()[0] # order of torsion
352361
self.__rank = self.__nfu + self.__nsu
353362

354-
# Move the torsion unit first, then fundamental units then S-units
355-
gens = [K(u, check=False) for u in su_fu_tu]
356-
gens = [gens[-1]] + gens[self.__nsu:-1] + gens[:self.__nsu]
363+
# Put the torsion unit first, then fundamental units then S-units
364+
gens = [K(pK.bnf_get_tu()[1], check=False)] + fu + su
357365

358366
# Construct the abstract group:
359367
gens_orders = tuple([ZZ(self.__ntu)]+[ZZ(0)]*(self.__rank))

0 commit comments

Comments
 (0)