Opened 3 years ago
Closed 7 months ago
#28345 closed defect (fixed)
Bug with NumberField.abs_val at zero
Reported by: | mercatp | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.5 |
Component: | number fields | Keywords: | |
Cc: | slelievre, cremona, roed, jhpalmieri | Merged in: | |
Authors: | Frédéric Chapoton | Reviewers: | Samuel Lelièvre |
Report Upstream: | N/A | Work issues: | |
Branch: | 1d7248c (Commits, GitHub, GitLab) | Commit: | 1d7248caaf3854bbbb64725b2eadc73986568b2d |
Dependencies: | Stopgaps: |
Description (last modified by )
The following code fails:
sage: x = polygen(ZZ) sage: K = NumberField(x^3 - 3, 'a') sage: v = tuple(K.primes_above(3))[0] sage: K.abs_val(v, K.zero()) Traceback (most recent call last) ... TypeError: unsupported operand parent(s) for ^: 'The Infinity Ring' and 'The Infinity Ring'
It should return 0. We fix the bug and add a doctest.
Related:
- #28241: Fix number field abs_val at non-real places
Change History (17)
comment:1 Changed 3 years ago by
- Summary changed from Bug with NumberField.abs_val to Bug with NumberField.abs_val at zero
comment:2 Changed 3 years ago by
- Branch set to u/mercatp/bug_with_numberfield_abs_val_at_zero
comment:3 Changed 3 years ago by
- Commit set to 9261c0ba965cb5c5dc8e5ab1090cb073d3da52b7
- Description modified (diff)
comment:4 Changed 3 years ago by
- Status changed from new to needs_review
I set this to needs review as it seems it is done, so the patchbots can check it.
The other way to do this would be to ensure that p^(-Infinity)
returns 0 in some way, but I don't suppose that really matters.
comment:5 Changed 2 years ago by
Returning 0
gives an int
which is of the wrong type, so please change it to return R.zero()
instead. Moreover, a blank line is needed after TESTS::
and please remove the trailing whitespace in the line above that. Also enter your full name in the authors field.
I am not too familiar with the theory, so it would be good if someone can confirm this is mathematically correct.
comment:6 Changed 2 years ago by
- Milestone changed from sage-8.9 to sage-9.0
- Status changed from needs_review to needs_work
comment:7 Changed 2 years ago by
- Milestone changed from sage-9.0 to sage-9.1
Ticket retargeted after milestone closed
comment:8 Changed 23 months ago by
- Milestone changed from sage-9.1 to sage-9.2
Moving tickets to milestone sage-9.2 based on a review of last modification date, branch status, and severity.
comment:9 Changed 17 months ago by
- Milestone changed from sage-9.2 to sage-9.3
comment:10 Changed 13 months ago by
- Milestone changed from sage-9.3 to sage-9.4
Setting new milestone based on a cursory review of ticket status, priority, and last modification date.
comment:11 Changed 8 months ago by
- Cc slelievre added
- Description modified (diff)
comment:12 Changed 8 months ago by
- Branch changed from u/mercatp/bug_with_numberfield_abs_val_at_zero to u/chapoton/28345
- Cc cremona roed added
- Commit changed from 9261c0ba965cb5c5dc8e5ab1090cb073d3da52b7 to 1d7248caaf3854bbbb64725b2eadc73986568b2d
- Status changed from needs_work to needs_review
here is a brand new branch, please review
New commits:
1d7248c | fix absolute value of zero in number fields
|
comment:13 Changed 8 months ago by
- Cc jhpalmieri added
comment:14 Changed 8 months ago by
green bot, please review this very simple ticket
comment:15 Changed 8 months ago by
- Reviewers set to Samuel Lelièvre
- Status changed from needs_review to positive_review
comment:16 Changed 7 months ago by
- Milestone changed from sage-9.4 to sage-9.5
comment:17 Changed 7 months ago by
- Branch changed from u/chapoton/28345 to 1d7248caaf3854bbbb64725b2eadc73986568b2d
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Correct a bug with NumberField.abs_val and add a doctest to check that it's corected