Opened 8 years ago

Closed 8 years ago

# conversion from maxima buggy

Reported by: Owned by: Ralf Stephan major sage-6.6 interfaces maxima, conversion, variable Karl-Dieter Crisman 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 Ralf Stephan

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 Ralf Stephan (previous) (diff)

### comment:3 Changed 8 years ago by Karl-Dieter Crisman

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 Ralf Stephan

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

### comment:5 follow-up:  6 Changed 8 years ago by Karl-Dieter Crisman

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 Ralf Stephan

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 Ralf Stephan

Branch: → u/rws/conversion_from_maxima_buggy

### comment:8 Changed 8 years ago by Ralf Stephan

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 Karl-Dieter Crisman

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 Karl-Dieter Crisman

```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 Nils Bruin

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 Nils Bruin

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 Ralf Stephan

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 Nils Bruin

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
```

It seems that changing the entry for `symtable['%e']` to `'exp(1)'` sort-of fixes this, but obviously, for `%i` and `%I` we have the same problem.

This won't be quite bulletproof either, due to:

```sage: function('log')
log
sage: sage.functions.log.log(x) == log(x)
```

but that currently won't even make it *to* maxima (that would need a `_SAGE_FUNCTION_log` encoding)

Last edited 8 years ago by Nils Bruin (previous) (diff)

### comment:15 Changed 8 years ago by Nils Bruin

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 Nils Bruin (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 Ralf Stephan

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 Nils Bruin

```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 Jeroen Demeyer

This was reported again at #17187.

### comment:20 Changed 8 years ago by Karl-Dieter Crisman

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

### comment:21 Changed 8 years ago by Ralf Stephan

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

### comment:22 Changed 8 years ago by Ralf Stephan

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 Nils Bruin

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 Ralf Stephan

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