Opened 2 years ago

Closed 3 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:

Status badges

Description (last modified by slelievre)

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 2 years ago by mercatp

  • Summary changed from Bug with NumberField.abs_val to Bug with NumberField.abs_val at zero

comment:2 Changed 2 years ago by mercatp

  • Branch set to u/mercatp/bug_with_numberfield_abs_val_at_zero

comment:3 Changed 2 years ago by mercatp

  • Commit set to 9261c0ba965cb5c5dc8e5ab1090cb073d3da52b7
  • Description modified (diff)

New commits:

9261c0bCorrect a bug with NumberField.abs_val and add a doctest to check that it's corected

comment:4 Changed 2 years ago by alexjbest

  • 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 gh-mwageringel

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 gh-mwageringel

  • 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 embray

  • Milestone changed from sage-9.0 to sage-9.1

Ticket retargeted after milestone closed

comment:8 Changed 19 months ago by mkoeppe

  • 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 14 months ago by mkoeppe

  • Milestone changed from sage-9.2 to sage-9.3

comment:10 Changed 10 months ago by mkoeppe

  • 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 5 months ago by slelievre

  • Cc slelievre added
  • Description modified (diff)

comment:12 Changed 4 months ago by chapoton

  • 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:

1d7248cfix absolute value of zero in number fields

comment:13 Changed 4 months ago by chapoton

  • Cc jhpalmieri added

comment:14 Changed 4 months ago by chapoton

green bot, please review this very simple ticket

comment:15 Changed 4 months ago by slelievre

  • Authors set to Frédéric Chapoton
  • Reviewers set to Samuel Lelièvre
  • Status changed from needs_review to positive_review

comment:16 Changed 3 months ago by mkoeppe

  • Milestone changed from sage-9.4 to sage-9.5

comment:17 Changed 3 months ago by vbraun

  • Branch changed from u/chapoton/28345 to 1d7248caaf3854bbbb64725b2eadc73986568b2d
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.