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

symbolic placeholder for complex root #22024

Closed
rwst opened this issue Dec 5, 2016 · 15 comments
Closed

symbolic placeholder for complex root #22024

rwst opened this issue Dec 5, 2016 · 15 comments

Comments

@rwst
Copy link
Contributor

rwst commented Dec 5, 2016

The Sage equivalent of SymPy's CRootOf (=ComplexRootOf) is missing, which is just a symbolic placeholder for a solution that cannot be displayed symbolically.

sage: from sympy import solve as ssolve
sage: ssolve(x^6+x+1, x)

[CRootOf(x**6 + x + 1, 0),
 CRootOf(x**6 + x + 1, 1),
 CRootOf(x**6 + x + 1, 2),
 CRootOf(x**6 + x + 1, 3),
 CRootOf(x**6 + x + 1, 4),
 CRootOf(x**6 + x + 1, 5)]
sage: (_[0]+1)._sage_()
...
AttributeError: 'ComplexRootOf' object has no attribute '_sage_'

Defect because conversion from SymPy fails.

A possible solution for #11941 depends on this.

Depends on #24062

Component: symbolics

Author: Ralf Stephan

Branch: ba171aa

Reviewer: Emmanuel Charpentier

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

@rwst rwst added this to the sage-7.5 milestone Dec 5, 2016
@nbruin
Copy link
Contributor

nbruin commented Dec 6, 2016

comment:1

I think it's a bit more than a symbolic placeholder. It does seem there's an embedding in CC associated with it. So conversion to sage should probably be by converting to QQbar. Something like this might work:

QQbar.polynomial_root(ZZ['x'](c.poly.as_expr()._sage_()),CIF(c.n()._sage_()))

but there will be a lot of corner cases to deal with too.

@rwst
Copy link
Contributor Author

rwst commented Nov 1, 2017

comment:2

Replying to @nbruin:

I think it's a bit more than a symbolic placeholder. It does seem there's an embedding in CC associated with it. So conversion to sage should probably be by converting to QQbar. Something like this might work:

QQbar.polynomial_root(ZZ['x'](c.poly.as_expr()._sage_()),CIF(c.n()._sage_()))

but there will be a lot of corner cases to deal with too.

To return a QQbar element would certainly be ideal. However, we're dealing at the moment with an unnecessary error, and returning a list of objects that show the polynomial plus the float value (and from which both can be extracted) would be sufficient to fix that defect.

@rwst
Copy link
Contributor Author

rwst commented Nov 1, 2017

comment:3

To return a float with arbitrary precision we can use SymPy's n() as well, so we just keep the original SymPy object.

@rwst
Copy link
Contributor Author

rwst commented Nov 1, 2017

@rwst
Copy link
Contributor Author

rwst commented Nov 1, 2017

Commit: ba171aa

@rwst
Copy link
Contributor Author

rwst commented Nov 1, 2017

Author: Ralf Stephan

@rwst
Copy link
Contributor Author

rwst commented Nov 1, 2017

Dependencies: #24062

@rwst rwst modified the milestones: sage-7.5, sage-8.1 Nov 1, 2017
@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Nov 14, 2017

comment:6

Builds, passes ptestlong with no error whatsoever.

I'm a bit stimyed by the usefulness of the result. If this is supposed to just allow the conversion of a SymPy computation into Sage, it does the job :

sage: sage: from sympy import solve as ssolve
....: sage: foo=ssolve(x^6+x+1, x)
....: 
sage: [t._sage_() for t in foo]

[complex_root_of(x^6 + x + 1, 0),
 complex_root_of(x^6 + x + 1, 1),
 complex_root_of(x^6 + x + 1, 2),
 complex_root_of(x^6 + x + 1, 3),
 complex_root_of(x^6 + x + 1, 4),
 complex_root_of(x^6 + x + 1, 5)]
sage: [t._sage_().n() for t in foo]

[-0.790667188814418 - 0.300506920309552*I,
 -0.790667188814418 + 0.300506920309552*I,
 -0.154735144496843 - 1.03838075445846*I,
 -0.154735144496843 + 1.03838075445846*I,
 0.945402333311260 - 0.611836693781009*I,
 0.945402333311260 + 0.611836693781009*I]

bit, as Nil's remarked, I find that we loose the properties of an answer in QQbar. However, the guarantee oif a fixed order may be the key to further computations.

==>positive_review

But see my forthcoming comment to #11941 : we may have bigger fish to fry...

@fchapoton
Copy link
Contributor

comment:7

reviewer name, please

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Nov 15, 2017

Reviewer: Emmanuel Charpentier

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Nov 15, 2017

comment:8

Replying to @fchapoton:

reviewer name, please

Wups ! Fixed...

@rwst
Copy link
Contributor Author

rwst commented Nov 16, 2017

comment:9

Replying to @EmmanuelCharpentier:

I'm a bit stimyed by the usefulness of the result.

I like such minimal solutions. I mean if we're importing sympy we might as well use their code. Thanks for the review.

@vbraun
Copy link
Member

vbraun commented Dec 11, 2017

Changed branch from u/rws/symbolic_placeholder_for_complex_root to ba171aa

@jdemeyer
Copy link
Contributor

comment:11

This is breaking on some patchbots: #24378

@jdemeyer
Copy link
Contributor

Changed commit from ba171aa 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