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.

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

Extract roots in NumberField if possible #25218

Closed
BrentBaccala opened this issue Apr 20, 2018 · 15 comments
Closed

Extract roots in NumberField if possible #25218

BrentBaccala opened this issue Apr 20, 2018 · 15 comments

Comments

@BrentBaccala
Copy link

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)
sqrt(2*I)
sage: I^(2/3)
I^(2/3)

New code:

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

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.

CC: @slel

Component: algebra

Keywords: NumberField

Author: Brent Baccala

Branch: 17b93d6

Reviewer: Sébastien Labbé

Issue created by migration from https://trac.sagemath.org/ticket/25218

@BrentBaccala BrentBaccala added this to the sage-8.2 milestone Apr 20, 2018
@BrentBaccala
Copy link
Author

Author: Brent Baccala

@BrentBaccala
Copy link
Author

New commits:

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

@BrentBaccala
Copy link
Author

Commit: 087174b

@BrentBaccala
Copy link
Author

Branch: u/gh-BrentBaccala/25218

@slel
Copy link
Member

slel commented Apr 20, 2018

comment:2

Regarding the suggestion that

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

see also #18036.

@slel
Copy link
Member

slel commented Apr 20, 2018

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 20, 2018

Changed commit from 087174b to 592e263

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 20, 2018

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

592e263Trac #25218: fix typo

@seblabbe
Copy link
Contributor

Reviewer: Sébastien Labbé

@seblabbe
Copy link
Contributor

Changed commit from 592e263 to 17b93d6

@seblabbe
Copy link
Contributor

comment:6

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


New commits:

17b93d625218: space fixes

@seblabbe
Copy link
Contributor

Changed branch from u/gh-BrentBaccala/25218 to u/slabbe/25218

@loefflerd loefflerd mannequin modified the milestones: sage-8.2, sage-8.3 May 17, 2018
@vbraun
Copy link
Member

vbraun commented May 18, 2018

Changed branch from u/slabbe/25218 to 17b93d6

@mezzarobba
Copy link
Member

Replying to @BrentBaccala:

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.

I think this is very unfortunate. In Sage, QQ[i] automatically comes with a complex embedding, for which i2/3 (= exp(iπ/3)) is perfectly well defined. It is really confusing to have a basic operator like ^ (whose evaluation normally uses the coercion framework) that is incompatible with the embedding.

See #30783.

@mezzarobba
Copy link
Member

Changed commit from 17b93d6 to none

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants