Opened 3 years ago

Closed 21 months ago

Last modified 21 months ago

#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: Changed 3 years ago by 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.

comment:2 in reply to: ↑ 1 Changed 23 months ago by rws

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 rws

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 rws

  • Branch set to u/rws/symbolic_placeholder_for_complex_root

comment:5 Changed 23 months ago by rws

  • Authors set to Ralf Stephan
  • 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: Changed 22 months ago by charpent

  • 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: Changed 22 months ago by chapoton

reviewer name, please

comment:8 in reply to: ↑ 7 Changed 22 months ago by charpent

  • Reviewers set to Emmanuel Charpentier

Replying to chapoton:

reviewer name, please

Wups ! Fixed...

comment:9 in reply to: ↑ 6 Changed 22 months ago by rws

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 vbraun

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

  • Commit ba171aa7029e35a6c444691376cad801e082ab36 deleted

This is breaking on some patchbots: #24378

Note: See TracTickets for help on using tickets.