Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use factored forms of number field units #31754

Open
videlec opened this issue Apr 29, 2021 · 1 comment
Open

Use factored forms of number field units #31754

videlec opened this issue Apr 29, 2021 · 1 comment
Labels
c: number fields p: major / 3 t: enhancement

Comments

@videlec
Copy link
Contributor

videlec commented Apr 29, 2021

PARI/GP 2.13.0 introduced factored forms for units of number fields. In the PARI/GP upgrade ticket #30801 we use some workaround to expand this factored form (via bnf.nfbasistoalg(bnf.nffactorback(.))). However, it would be much more natural (and efficient) if Sage worked directly with the factored form.

Depends on #30801

CC: @slel

Component: number fields

Issue created by migration from https://trac.sagemath.org/ticket/31754
@videlec videlec added this to the sage-9.4 milestone Apr 29, 2021
@slel

This comment has been minimized.

Sign in to view
@slel slel changed the title used factored forms of number field units Use factored forms of number field units Apr 29, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Aug 10, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 1, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Aug 31, 2022
@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
@slel slel mentioned this issue Jul 2, 2021
Closed
This was referenced Feb 1, 2024
Closed
Merged
vbraun pushed a commit to vbraun/sage that referenced this issue Feb 11, 2024
sagemathgh-37226: unit group of number field: do not expand product w…
705cab1 
…hen 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 sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

This PR fixes sagemath#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
sagemath#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 sagemath#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
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#37226
Reported by: AurelPage
Reviewer(s): AurelPage, Vincent Delecroix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: number fields p: major / 3 t: enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@slel @videlec @mkoeppe