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

Bug in solve due to a bug in symbolic_expression_from_maxima_string #20755

Closed
bgrenet opened this issue Jun 1, 2016 · 8 comments
Closed

Bug in solve due to a bug in symbolic_expression_from_maxima_string #20755

bgrenet opened this issue Jun 1, 2016 · 8 comments

Comments

@bgrenet
Copy link

bgrenet commented Jun 1, 2016

A bug in sage.calculus.calculus.symbolic_expression_from_maxima_string implies bugs in solve and roots for symbolic expressions.

Symptoms

The method solve for symbolic expressions is buggy (the list of multiplicities has size 2 instead of 4), as well as roots as a (serious!) consequence:

sage: w = x^4 - (1+3*i)*x^3 - (2-4*i)*x^2 + (6-2*i)*x - 4 - 4*i
sage: w.solve(x,multiplicities=True)
([x == -1/2*sqrt(2*I) + 3/2*I - 1/2, x == 1/2*sqrt(2*I) + 3/2*I - 1/2, x == (-I + 1), x == (I + 1)],
 [1, 1])
sage: w.roots() # should be 4 roots
[(-1/2*sqrt(2*I) + 3/2*I - 1/2, 1), (1/2*sqrt(2*I) + 3/2*I - 1/2, 1)]

Diagnosis

  1. The behavior of roots is easily explained by the behavior of solve, since roots assume that the length of the list of solutions is the same as the length of the list of multiplicities. This should clearly be the case so the bug is not in roots.

  2. Given the parameter in the example, solve calls Maxima and parses the result. The multiplicities are obtained by invoking P.get('multiplicities'). This is the right invocation to Maxima, no bug there.

  3. To parse the solutions returned by Maxima, solve calls sage.symbolic.relation.string_to_list_of_solutions, which itself calls sage.calculus.symbolic_expression_from_maxima_string. The bug occurs in this last function: Indeed, while there is no apparent reason for this, invoking this function changes the variable multiplicities of Maxima. Here is an example:

    sage: w = x^4 - (1+3*i)*x^3 - (2-4*i)*x^2 + (6-2*i)*x - 4 - 4*i
    sage: m = (w == 0)._maxima_()
    sage: P = m.parent()
    sage: s = m.solve(x).str()
    sage: s # the list of solutions returned by Maxima
    '[_SAGE_VAR_x=-(sqrt(2*%i)-3*%i+1)/2,_SAGE_VAR_x=(sqrt(2*%i)+3*%i-1)/2,_SAGE_VAR_x=1-%i,_SAGE_VAR_x=%i+1]'
    sage: P.get('multiplicities') # correct!
    '[1,1,1,1]'
    sage: l = sage.calculus.calculus.symbolic_expression_from_maxima_string(s)
    sage: l
    [x == -1/2*sqrt(2*I) + 3/2*I - 1/2,
     x == 1/2*sqrt(2*I) + 3/2*I - 1/2,
     x == (-I + 1),
     x == (I + 1)]
    sage: P.get('multiplicities') # WTF???
    '[1,1]'

Component: **symbolics**

Keywords: **maxima, solve**

Author: **Bruno Grenet**

Branch/Commit: **[`eb0ad68`](https://github.com/sagemath/sagetrac-mirror/commit/eb0ad68ae007807828945e13e58e9865c67adf77)**

Reviewer: **Vincent Delecroix**

_Issue created by migration from https://trac.sagemath.org/ticket/20755_

@bgrenet bgrenet added this to the sage-7.3 milestone Jun 1, 2016
@nbruin
Copy link
Contributor

nbruin commented Jun 1, 2016

comment:2

Well, multiplicities is a system variable. See http://maxima.sourceforge.net/docs/manual/maxima_20.html. Who knows when it gets updated? In particular, the reconstruction of symbolic expressions from strings might be calling into maxima again.

The quick fix is to get the correct value of multiplicities out as soon as we can, before conversion back.

In the longer run, it would be preferable to do the conversion while avoiding strings altogether. See sr_sum and sr_limit etc. for examples how one can use sr_to_max and max_to_sr to convert.

@fchapoton
Copy link
Contributor

comment:3

seems to work in 8.9.b1

one needs to check and add a doctest

@fchapoton fchapoton modified the milestones: sage-7.3, sage-8.9 Jul 9, 2019
@bgrenet
Copy link
Author

bgrenet commented Aug 21, 2019

Commit: eb0ad68

@bgrenet
Copy link
Author

bgrenet commented Aug 21, 2019

comment:4

The bug seems indeed fixed. I've add a doctest.


New commits:

eb0ad6820755: Add doctest

@bgrenet
Copy link
Author

bgrenet commented Aug 21, 2019

Author: Bruno Grenet

@bgrenet
Copy link
Author

bgrenet commented Aug 21, 2019

Branch: u/bruno/20755_bug_in_solve

@videlec
Copy link
Contributor

videlec commented Aug 21, 2019

Reviewer: Vincent Delecroix

@vbraun
Copy link
Member

vbraun commented Aug 29, 2019

Changed branch from u/bruno/20755_bug_in_solve to eb0ad68

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