#22024 closed defect (fixed)
symbolic placeholder for complex root
Reported by: | rws | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.1 |
Component: | symbolics | Keywords: | |
Cc: | Merged in: | ||
Authors: | Ralf Stephan | Reviewers: | Emmanuel Charpentier |
Report Upstream: | N/A | Work issues: | |
Branch: | ba171aa (Commits) | Commit: | |
Dependencies: | #24062 | Stopgaps: |
Description
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.
Change History (11)
comment:1 follow-up: ↓ 2 Changed 3 years ago by
comment:2 in reply to: ↑ 1 Changed 23 months ago by
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.
comment:3 Changed 23 months ago by
To return a float with arbitrary precision we can use SymPy's n()
as well, so we just keep the original SymPy object.
comment:4 Changed 23 months ago by
- Branch set to u/rws/symbolic_placeholder_for_complex_root
comment:5 Changed 23 months ago by
- Commit set to ba171aa7029e35a6c444691376cad801e082ab36
- Dependencies set to #24062
- Milestone changed from sage-7.5 to sage-8.1
- Status changed from new to needs_review
comment:6 follow-up: ↓ 9 Changed 22 months ago by
- Status changed from needs_review to positive_review
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...
comment:7 follow-up: ↓ 8 Changed 22 months ago by
reviewer name, please
comment:8 in reply to: ↑ 7 Changed 22 months ago by
- Reviewers set to Emmanuel Charpentier
comment:9 in reply to: ↑ 6 Changed 22 months ago by
Replying to charpent:
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.
comment:10 Changed 21 months ago by
- Branch changed from u/rws/symbolic_placeholder_for_complex_root to ba171aa7029e35a6c444691376cad801e082ab36
- Resolution set to fixed
- Status changed from positive_review to closed
comment:11 Changed 21 months ago by
- Commit ba171aa7029e35a6c444691376cad801e082ab36 deleted
This is breaking on some patchbots: #24378
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:
but there will be a lot of corner cases to deal with too.