Opened 4 years ago

Closed 3 years ago

Improper expressions from SR(string)

Reported by: Owned by: rws major sage-8.1 symbolics FriCAS mantepse Frédéric Chapoton Martin Rubey N/A f9b0635 (Commits) f9b0635bc6f56595c16c4db99ccc6733270dc2b7

Strings are not translated into proper Sage expressions for some functions:

```sage: SR("sin(x)")
sin(x)
sage: type(_.operator())
<class 'sage.functions.trig.Function_sin'>
sage: SR("arcsin(x)")
arcsin(x)
sage: type(_.operator())
<class 'sage.functions.trig.Function_arcsin'>
sage: SR("asin(x)")
asin(x)
sage: type(_.operator())
<class 'sage.symbolic.function_factory.NewSymbolicFunction'>
```

Previous ticket description:

```sage: integrate(((-1-3*x)^(-1/2)*(-1+3*x)^(-1/2)),x,algorithm='fricas')
-2/3*atan(1/3*(sqrt(3*x - 1)*sqrt(-3*x - 1) - 1)/x)
sage: _.subs(x==.1)
-2/3*atan(-6.51313067138982 + 3.89412863198815e-16*I)
sage: _.n()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-29-875d0b089ddd> in <module>()
----> 1 _.n()

/home/ralf/sage/src/sage/structure/element.pyx in sage.structure.element.Element.n (build/cythonized/sage/structure/element.c:7987)()
836             0.666666666666667
837         """
--> 838         return self.numerical_approx(prec, digits, algorithm)
839
840     N = deprecated_function_alias(13055, n)

/home/ralf/sage/src/sage/symbolic/expression.pyx in sage.symbolic.expression.Expression.numerical_approx (build/cythonized/sage/symbolic/expression.cpp:33844)()
5566             res = x.pyobject()
5567         else:
-> 5568             raise TypeError("cannot evaluate symbolic expression numerically")
5569
5570         # Important -- the  we get might not be a valid output for numerical_approx in

TypeError: cannot evaluate symbolic expression numerically
```

Already the second command should have evaluated to a floating point value.

comment:1 Changed 4 years ago by mantepse

A simpler way to see the bug:

```sage: fricas(atan(x)).sage() == atan(x)
atan(x) == arctan(x)
```

comment:2 Changed 4 years ago by mantepse

I don't quite get what's happening. Is there something wrong with my "a"?

```sage: [f(x)._fricas_().sage().subs(x=1.0) for f in [sin, cos, sec, csc, cot, tan, asin, acos,
....: atan, acot, acsc, asec, arcsin, arccos, arctan, arccot, arccsc, arcsec]]
[0.841470984807897,
0.540302305868140,
1.85081571768093,
1.18839510577812,
0.642092615934331,
1.55740772465490,
asin(1.00000000000000),
acos(1.00000000000000),
atan(1.00000000000000),
acot(1.00000000000000),
acsc(1.00000000000000),
asec(1.00000000000000),
asin(1.00000000000000),
acos(1.00000000000000),
atan(1.00000000000000),
acot(1.00000000000000),
acsc(1.00000000000000),
asec(1.00000000000000)]
sage: [f(x)._fricas_().sage().subs(x=1.0) for f in [tanh, sinh, cosh, coth, sech, csch, asinh,
....:  acosh, atanh, acoth, asech, acsch, arcsinh, arccosh, arctanh, arccoth, arcsech, arccsch
....: ]]

[0.761594155955765,
1.17520119364380,
1.54308063481524,
1.31303528549933,
0.648054273663885,
0.850918128239322,
asinh(1.00000000000000),
acosh(1.00000000000000),
atanh(1.00000000000000),
acoth(1.00000000000000),
asech(1.00000000000000),
acsch(1.00000000000000),
asinh(1.00000000000000),
acosh(1.00000000000000),
atanh(1.00000000000000),
acoth(1.00000000000000),
asech(1.00000000000000),
acsch(1.00000000000000)]
```

comment:3 Changed 4 years ago by mantepse

I just learned:

```sage: type(fricas(atan(x)).sage().operator())
<class 'sage.symbolic.function_factory.NewSymbolicFunction'>

sage: type(atan(x).operator())
<class 'sage.functions.trig.Function_arctan'>

sage: type(fricas(sin(x)).sage().operator())
<class 'sage.functions.trig.Function_sin'>
```

but I do not know where this `NewSymbolicFunction` thing comes from, and why it only appears for `atan`.

comment:4 Changed 4 years ago by rws

• Description modified (diff)
• Keywords FriCAS removed
• Summary changed from Improper expressions from FriCAS interface to Improper expressions from SR(string)

Interesting. It's the conversion via `SR(string)`.

comment:5 Changed 4 years ago by rws

• Description modified (diff)

In `calculus/calculus.py:symbolic_expression_from_string()` the `arc` versions are recognized but not the `a` versions.

comment:6 Changed 4 years ago by rws

However if I put these into the global `_augmented` dict (as string/function pairs) the dict will be empty when doctesting via `sage -tp`. Can someone explain this to a Python amateur?

comment:7 Changed 4 years ago by mantepse

not sure whether the following is helpful:

```sage: from sage.libs.pynac.pynac import symbol_table
sage: type(symbol_table["functions"]["sin"])
<class 'sage.functions.trig.Function_sin'>

sage: type(symbol_table["functions"]["asin"])
<class 'sage.symbolic.function_factory.NewSymbolicFunction'>

sage: type(symbol_table["functions"]["arcsin"])
<class 'sage.functions.trig.Function_arcsin'>
```

comment:8 Changed 4 years ago by mantepse

Sorry, mistake: in a fresh session we have

```sage: from sage.libs.pynac.pynac import symbol_table
sage: type(symbol_table["functions"]["asin"])
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
<ipython-input-3-95eddacf035c> in <module>()
----> 1 type(symbol_table["functions"]["asin"])

KeyError: 'asin'
```

comment:9 Changed 4 years ago by mantepse

Is it possible that `SR("asin")`, `SR("atan")`, etc. actually are supposed to fail?

I don't know how the "conversion" machinery is supposed to work, but `symbol_table["fricas"]` does contain the correct translations. For example:

```sage: from sage.libs.pynac.pynac import symbol_table
sage: symbol_table["fricas"]["asin"]
arcsin

sage: type(_)
<class 'sage.functions.trig.Function_arcsin'>
```

Also, it does seem to be a problem with the fricas interface:

```sage: type(maxima("asin(x)").sage().operator())
<class 'sage.functions.trig.Function_arcsin'>
sage: type(fricas("asin(x)").sage().operator())
<class 'sage.symbolic.function_factory.NewSymbolicFunction'>
```

In fact, `maxima_abstract.py:_sage_` contains (skipping the docstring)

```    def _sage_(self):
"""
...
"""
import sage.calculus.calculus as calculus
return calculus.symbolic_expression_from_maxima_string(self.name(),
maxima=self.parent())
```

which looks more plausible than the `fricas.py` counterpart (which I wrote...):

```        from sage.symbolic.ring import SR
s = unparsed_InputForm
replacements = [('pi()', 'pi '),
('::Symbol', ' ')]
for old, new in replacements:
s = s.replace(old, new)
try:
return SR(s)
except TypeError:
raise NotImplementedError("The translation of the FriCAS Expression %s to sage is not yet implemented." %s)
```

comment:10 Changed 4 years ago by mantepse

I'm now pretty much convinced that it is wrongdoing of the FriCAS interface. An essential step in `symbolic_expression_from_maxima_string` is to replace all known functions with their sage equivalent, using `symbol_table`.

So, we should do the following: rewrite `fricas._sage_expression`, which currently takes the `unparsed InputForm` of a FriCAS `Expression Integer` or `OrderedCompletion Expression Integer` object as follows:

• input should be the `InputForm`, instead of the `unparsed InputForm`, which is essentially a nested list of the form `(function arg1 arg2 ...)` (lisp syntax)
• replace function by its sage equivalent, and recurse.

I am unlikely to do that.

comment:12 Changed 4 years ago by rws

I think your analysis didn't consider the fact that `return SR(s)` in `fricas.py` calls `symbolic_expression_from_string` and therefore `symbolic_expression_from_string` would be the code to fix.

comment:13 Changed 4 years ago by mantepse

Yes, but it shouldn't. `symbolic_expression_from_string` hopes for expressions in terms of the dictionary `symbol_table["functions"]`, not `symbol_table["fricas"]`. I didn't know about `symbol_table` when I coded the interface.

comment:14 Changed 3 years ago by chapoton

• Branch set to public/22525
• Commit set to 77753048d3778fc4c286d1ed689e0502fc58e2d4
• Status changed from new to needs_info

Here is a tentative of enhancement.

New commits:

 ​7775304 `trac 22525 better conversion from fricas expressions`

comment:15 Changed 3 years ago by chapoton

• Milestone changed from sage-7.6 to sage-8.1

comment:16 Changed 3 years ago by git

• Commit changed from 77753048d3778fc4c286d1ed689e0502fc58e2d4 to f9b0635bc6f56595c16c4db99ccc6733270dc2b7

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

 ​f9b0635 `add a test`

comment:17 Changed 3 years ago by mantepse

This looks great! I added a test checking that we can use it to compute numerical values, which is probably the most interesting part of the fix.

Unfortunately, I have to recompile almost from scratch. I'll set it to positive review afterwards.

comment:18 Changed 3 years ago by mantepse

• Authors set to Frédéric Chapoton
• Reviewers set to Martin Rubey
• Status changed from needs_info to needs_review

comment:19 Changed 3 years ago by mantepse

(most of the tests will fail here, because I made a mistake in #22501, which is fixed in #23782)

Version 0, edited 3 years ago by mantepse (next)

comment:20 Changed 3 years ago by mantepse

• Status changed from needs_review to positive_review

Looks good!

comment:21 Changed 3 years ago by vbraun

• Branch changed from public/22525 to f9b0635bc6f56595c16c4db99ccc6733270dc2b7
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.