Opened 8 years ago

Closed 8 years ago

# conversion from maxima buggy

Reported by: Owned by: rws major sage-6.6 interfaces maxima, conversion, variable kcrisman Nils Bruin Ralf Stephan N/A ed8d3b1

### Description

On Wed, Aug 27, 2014 at 2:46 PM, Peter Mueller <ypf...@googlemail.com> wrote:
> My understanding of Sage is that var('e') declares e as a symbolic variable,
> no matter that it was the Euler number before. The last line leaves me
> clueless what goes wrong ...
>
> sage: var('e')
> e
> sage: matrix.diagonal([e,1,1]).det()
> e
> sage: matrix.diagonal([e,1,1,1]).det()
> _e

On Wednesday, August 27, 2014 3:13:58 PM UTC+2, Stein William wrote:
The real bug is in conversion from Maxima to Sage.  Observe:

~/wstein/sage-6.4.beta1\$ ./sage
sage: maxima('_SAGE_VAR_e')._sage_()
_e

The same, as suspected, with i, so #6882 is the culprit.

### comment:1 Changed 8 years ago by rws

Well, so sefms should know about existing Sage variables and prepend the underscore only if the variable doesn't exist:

sage: sefms('e')
_e
sage: e = var('e')
sage: sefms('e')
e

To recall, some marking is needed because removal of _SAGE_VAR_ has already happened before sefms, and e could well be a Maxima variable. We just didn't think that someone could use such Sage variables.

Last edited 8 years ago by rws (previous) (diff)

### comment:3 Changed 8 years ago by kcrisman

Aargh!!! But checking variables is often a nightmare, and there are other things that could be used for e too.

### comment:4 Changed 8 years ago by rws

But the other things are not shoved to Maxima and back, are they?

### comment:5 follow-up:  6 Changed 8 years ago by kcrisman

Well, anything in a symbolic matrix (or a symbolic integral, or limit) gets sent to Maxima and back. But maybe just raw variables are the only worry here.

The following would be the thing to fix.

sage: e = var('e')
sage: e == maxima(e)._sage_()
e == _e

whereas

sage: reset()
sage: e == maxima(e)._sage_()
e == e

### comment:6 in reply to:  5 ; follow-up:  9 Changed 8 years ago by rws

The following would be the thing to fix.

sage: e = var('e')
sage: e == maxima(e)._sage_()
e == _e

whereas

sage: reset()
sage: e == maxima(e)._sage_()
e == e

I don't think so. It should both be e == e because the first time you're giving the variable as argument, and the second time the constant. Rather:

sage: e = var('e')
sage: e == maxima(e)._sage_()
e == e
sage: reset()
sage: e == maxima(e)._sage_()
e == e
sage: e == maxima('e')._sage_()
e == _e

Fix proposal following.

### comment:7 Changed 8 years ago by rws

Branch: → u/rws/conversion_from_maxima_buggy

### comment:8 Changed 8 years ago by rws

Commit: → f977bad314fcce8bfb85d72564cc64545b71f1fb new → needs_review

It was much easier than feared because we could get to the string before removal of _SAGE_VAR_. The method is to mark specific maxima vars in the string with underscore. Note: this or a different prefix could be given to all maxima vars.

New commits:

 ​f977bad 16898: do not give sage vars the pre-underscore

### comment:9 in reply to:  6 Changed 8 years ago by kcrisman

The following would be the thing to fix.

I don't think so. It should both be e == e because the first time you're giving the variable as argument, and the second time the constant. Rather:

I was actually agreeing with you - it was the thing to FIX. :-)

Fix proposal following.

Okay, I'll look at this.

### comment:10 Changed 8 years ago by kcrisman

s[:m.start()-1] + '_' + s[m.start()]

should that maybe just be

s[:m.start()] + '_' + s[m.start()]

we're not getting rid of anything, are we? Just putting in an underscore? The documentation seems to suggest this, since Python slices have "the end always excluded". I don't think any of your examples catch that, since they just have e. But foo+bar+e might get turned to foo+bar_e? (Sorry for not trying this out quite yet, I don't have an easy-access branch just now.)

### comment:11 Changed 8 years ago by nbruin

Would it be possible to fix this properly? It should really be easier to do the conversion properly now that variables get converted to something with a _SAGE_VAR_ prefix.

From sage to maxima: At this point you know whether an object is a symbolic variable or a (predefined) symbolic constant. In one case you convert it to the string _SAGE_VAR_e, in the other case you convert it to %e.

From maxima to sage: This is the trickier one, because the strings-based interface needs to try to tell everything from string representations. However, since we're either seeing %e or _SAGE_VAR_e, there's not really any confusion. Whether a variable "e" exists doesn't even matter. Indeed, we have

sage: var('e')
e
sage: e==exp(1)
e == e
sage: bool(e==exp(1))
False
sage: (e==exp(1))._maxima_init_()
'_SAGE_VAR_e = exp(1)'

so we should make sure that %e gets converted back to sage.functions.log.exp(1) and that that _SAGE_VAR_e gets converted to SR.var('e').

The round trip currently goes horribly wrong:

sage: A=e==exp(1)
sage: A
sage: maxima_calculus(A) # this is fine
_SAGE_VAR_e=%e
sage: A_roundtrip=maxima_calculus(A)._sage_()
sage: A_roundtrip.lhs() #a new symbol got invented!
_e
sage: bool(A_roundtrip.rhs() == exp(1)) #and the right hand side is now a symbolic variable!
False
sage: bool(maxima_calculus(A)._sage_())
False
sage: bool(maxima_calculus(maxima_calculus(A)._sage_())._sage_()) #fun all the way.
True

If confusion arises, then this is probably due to inappropriate string substitutions. It may well be that sefms is up for a major refactoring.

I was a little surprised that max_to_sr and sr_to_max get this wrong too:

sage: max_to_sr(maxima_calculus(sr_to_max(A)).ecl())
e == e
sage: bool(max_to_sr(maxima_calculus(sr_to_max(A)).ecl()))
True

but this may be because currently, sr_to_max/max_to_sr get their initial translations from string-based. Certainly the infrastructure for these routines fully allows to distinguish objects regardless of their printed representation. These could be fixed by handling "symbolic variables" and "_SAGE_VAR_..." symbols (on the lisp side) separately (in sr_to_max/max_to_sr). That would reduce chances that we mess up the translation tables. In fact, objects of type sage.symbolic.function_factory.NewSymbolicFunction should probably be translated to _SAGE_FUNC_... in maxima for similar reasons.

### comment:12 Changed 8 years ago by nbruin

perhaps check out _find_var and _find_func, used in symbolic_expression_from_string (in the same file) rather than symbolic_expression_from_maxima_string. Those work on string tokens once the expression is actually parsed. Conversion there should be much more straightforward than by string-based preprocessing using regular expressions, which is what happens in symbolic_expression_from_maxima_string. Probably, anything we can do there rather than by dumb string manipulations will be much better and easier to maintain than the mess we have now. Certainly, making _find_var aware of _SAGE_VAR_... rather than just stripping out this valuable marker will lead to a much more efficient and reliable conversion process. I don't have the time to dive into the peculiarities of the relevant routines, but the code looks pretty straightforward, so hopefully someone is willing to invest a little time in refactoring this code. It will make working on strings-based maxima-to-sage conversions much more pleasurable.

### comment:13 follow-up:  14 Changed 8 years ago by rws

Well, I think it's better to apply this quick fix and work towards abandoning of the expect interface.

### comment:14 in reply to:  13 Changed 8 years ago by nbruin

Well, I think it's better to apply this quick fix and work towards abandoning of the expect interface.

The proposed fix doesn't solve the issue, though. I've tried the branch here and I get this:

sage: A = SR.var('e')==exp(1)
sage: A # the printing of this may be confusing but the meaning to sage is clear
e == e
sage: bool(A)
False
sage: bool(SR(maxima_calculus(A))) #the distinction doesn't survive the round-trip.
True
sage: A.rhs().is_symbol()
False
sage: SR(maxima_calculus(A)).rhs().is_symbol()
True

so it seems that the existence of a variable e in pynac causes maxima's %e to be translated to SR.var('e'). Illustrating this directly:

sage: from sage.calculus.calculus import symbolic_expression_from_maxima_string as sefms
sage: sefms('%e').is_symbol()
False
sage: SR.var('e') #apparently this affects the translation
e
sage: sefms('%e').is_symbol()
True
Version 1, edited 8 years ago by nbruin (previous) (next) (diff)

### comment:15 Changed 8 years ago by nbruin

Branch: u/rws/conversion_from_maxima_buggy → u/nbruin/conversion_from_maxima_buggy

New branch, based on doing more at parser level rather than at string mangling level. Previous branch was:

u/rws/conversion_from_maxima_buggy

I think the present branch is already more in the direction. Important obstacle to using Bradshaw's parser straight on maxima output: It can't handle % characters as part of identifiers (it's an operator in python after all!), I think the current branch solves at least the problem stated in the ticket without creating new problems.

Last edited 8 years ago by nbruin (previous) (diff)

### comment:16 Changed 8 years ago by git

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

 ​e6cd65b trac #16898: Use parser to distinguish between maxima internal and sage variable names

### comment:17 follow-up:  18 Changed 8 years ago by rws

Status: needs_review → needs_work

Buildbot reports some order changes in doctests of src/sage/matrix/matrix2.pyx. Also, I'm not sure if it fits the ticket, I found the following:

sage: pari.pollegendre(4,e)
35/8*e^4 - 15/4*e^2 + 3/8
sage: SR(_)
35/8*e^4 - 15/4*e^2 + 3/8
sage: _.simplify_full()
35/8*_e^4 - 15/4*_e^2 + 3/8

### comment:18 in reply to:  17 Changed 8 years ago by nbruin

sage: pari.pollegendre(4,e)
35/8*e^4 - 15/4*e^2 + 3/8
sage: SR(_)
35/8*e^4 - 15/4*e^2 + 3/8
sage: _.simplify_full()
35/8*_e^4 - 15/4*_e^2 + 3/8

Nice one. That's a separate ticket, I think, though:

sage: f=pari.pollegendre(4,e)
sage: g=SR(f)
sage: g.operator()
sage: g.operands()
[]
sage: g.pyobject() is f
True

Apparently pari "polynomials" don't get properly converted to SR, but just get stuffed in. Consequently, the sage-to-maxima conversion just sees if maxima can make sense of the string representation. For instance (and that's what you see) variables don't get properly converted:

sage: maxima_calculus(g) #note no _SAGE_VAR_ prefixes
35*e^4/8-15*e^2/4+3/8

sage: QQ['e'](g)
TypeError: Unable to coerce PARI 35/8*e^4 - 15/4*e^2 + 3/8 to an Integer

The truth is, almost everything can be stuffed in SR and, as a result, not everything in SR can be translated to maxima:

sage: M=pari.matrix(2,2)
sage: M
[0, 0; 0, 0]
sage: M.simplify_full()
AttributeError: 'MaximaLibElement' object has no attribute '_name'
sage: maxima_calculus(M) #maxima's reader chokes on [0,0;0,0] because it's ungrammatical in maximan
TypeError: ECL says: THROW: The catch MACSYMA-QUIT is undefined.

We can hide the bad behaviour in the particular example you gave by folding both _SAGE_VAR_e and e back onto e but you'd still get wrong answers:

sage: integrate(g,e)
integrate(35/8*e^4 - 15/4*e^2 + 3/8, e)
sage: integrate(g,e).simplify()
1/8*(35*_e^4 - 30*_e^2 + 3)*e

In the latter one, at least we now see something funny has happened. If it were to multiply out, it's truly confusing.

### comment:19 Changed 8 years ago by jdemeyer

This was reported again at #17187.

### comment:20 Changed 8 years ago by kcrisman

Thanks, Jeroen. So what needs fixing here is just some doctests?

### comment:21 Changed 8 years ago by rws

Branch: u/nbruin/conversion_from_maxima_buggy → public/16898_conversion_from_maxima_buggy

### comment:22 Changed 8 years ago by rws

Authors: → Nils Bruin e6cd65b2e11b88e57a19d993f75c9fec60af5c81 → ed8d3b1cd0744940bbf9ac851c3f3a762ca54083 → Ralf Stephan needs_work → needs_review

New commits:

 ​c38f654 Merge branch 'develop' into t/16898/conversion_from_maxima_buggy ​ed8d3b1 16898: fix failing doctests in matrix2

### comment:23 follow-up:  24 Changed 8 years ago by nbruin

I'm OK with the amended doctests, so if someone else is happy with the other changes, this ticket can be set to positive review.

### comment:24 in reply to:  23 Changed 8 years ago by rws

Milestone: sage-6.4 → sage-6.6 needs_review → positive_review