#25218 closed enhancement (fixed)

Extract roots in NumberField if possible

Reported by: gh-BrentBaccala Owned by:
Priority: minor Milestone: sage-8.3
Component: algebra Keywords: NumberField
Cc: slelievre Merged in:
Authors: Brent Baccala Reviewers: Sébastien Labbé
Report Upstream: N/A Work issues:
Branch: 17b93d6 (Commits) Commit: 17b93d604a5c2d45a136d171db513681a320f58f
Dependencies: Stopgaps:


NumberField previously evaluated integral powers in the NumberField, and evaluated all fractional powers in the symbolic ring.

This patch makes NumberField attempt to evaluate the fractional power within the field, and only falls back on the symbolic ring if this fails.

There's a few interesting changes in the test suite.

Old code:

sage: QQbar((2*I)^(1/2))
1 + 1*I
sage: (2*I)^(1/2)
sage: I^(2/3)

New code:

sage: QQbar((2*I)^(1/2))
I + 1
sage: (2*I)^(1/2)
I + 1
sage: I^(2/3)

The first change is just cosmetic. The second makes good sense, as Sage is now evaluating an expression it didn't before. The third change is more troubling.

The explanation lies in the definition of I:

sage: I.parent()
Symbolic Ring
sage: I.pyobject().parent()
Number Field in I with defining polynomial x^2 + 1

In this number field, there is a single cube root of I (-I). Squaring -I gives us -1, so I^(2/3)=-1.

My opinion is that the new behavior of NumberField is correct and preferred, but perhaps I should be defined in QQbar, not in a NumberField.

Change History (9)

comment:1 Changed 21 months ago by gh-BrentBaccala

  • Authors set to Brent Baccala
  • Branch set to u/gh-BrentBaccala/25218
  • Commit set to 087174bfe499b07ed66ea60c4f6eb2c9a0aee7a3

New commits:

75a46a0Trac #25218: NumberField attempts to evaluate fractional powers
087174bTrac #25218: update test cases altered by this patch

comment:2 Changed 21 months ago by slelievre

  • Cc slelievre added

Regarding the suggestion that

perhaps I should be defined in QQbar, not in a NumberField,

see also #18036.

comment:3 Changed 21 months ago by slelievre

The following tickets are possibly related.

comment:4 Changed 21 months ago by git

  • Commit changed from 087174bfe499b07ed66ea60c4f6eb2c9a0aee7a3 to 592e263482a364ed1987db67aa41c6f2897b8e95

Branch pushed to git repo; I updated commit sha1. New commits:

592e263Trac #25218: fix typo

comment:5 Changed 21 months ago by gh-BrentBaccala

  • Status changed from new to needs_review

comment:6 Changed 21 months ago by slabbe

  • Branch changed from u/gh-BrentBaccala/25218 to u/slabbe/25218
  • Commit changed from 592e263482a364ed1987db67aa41c6f2897b8e95 to 17b93d604a5c2d45a136d171db513681a320f58f
  • Reviewers set to Sébastien Labbé

I did small spaces fixes. If you agree with my changes, please change the status to positive review.

New commits:

17b93d625218: space fixes

comment:7 Changed 21 months ago by gh-BrentBaccala

  • Status changed from needs_review to positive_review

comment:8 Changed 21 months ago by davidloeffler

  • Milestone changed from sage-8.2 to sage-8.3

comment:9 Changed 21 months ago by vbraun

  • Branch changed from u/slabbe/25218 to 17b93d604a5c2d45a136d171db513681a320f58f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.