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

Improper expressions from SR(string) #22525

Closed
rwst opened this issue Mar 6, 2017 · 28 comments
Closed

Improper expressions from SR(string) #22525

rwst opened this issue Mar 6, 2017 · 28 comments

Comments

@rwst
Copy link

rwst commented Mar 6, 2017

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.

CC: @mantepse

Component: symbolics

Keywords: FriCAS

Author: Frédéric Chapoton

Branch/Commit: f9b0635

Reviewer: Martin Rubey

Issue created by migration from https://trac.sagemath.org/ticket/22525

@rwst rwst added this to the sage-7.6 milestone Mar 6, 2017
@mantepse
Copy link
Collaborator

mantepse commented Mar 7, 2017

Changed keywords from none to FriCAS

@mantepse
Copy link
Collaborator

mantepse commented Mar 7, 2017

comment:1

A simpler way to see the bug:

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

@mantepse
Copy link
Collaborator

mantepse commented Mar 7, 2017

comment:2

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)]

@mantepse
Copy link
Collaborator

mantepse commented Mar 7, 2017

comment:3

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.

@rwst
Copy link
Author

rwst commented Mar 8, 2017

Changed keywords from FriCAS to none

@rwst

This comment has been minimized.

@rwst
Copy link
Author

rwst commented Mar 8, 2017

comment:4

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

@rwst rwst changed the title Improper expressions from FriCAS interface Improper expressions from SR(string) Mar 8, 2017
@rwst

This comment has been minimized.

@rwst
Copy link
Author

rwst commented Mar 8, 2017

comment:5

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

@rwst
Copy link
Author

rwst commented Mar 8, 2017

comment:6

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?

@mantepse
Copy link
Collaborator

mantepse commented Mar 8, 2017

comment:7

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

@mantepse
Copy link
Collaborator

mantepse commented Mar 8, 2017

comment:8

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'

@mantepse
Copy link
Collaborator

mantepse commented Mar 8, 2017

comment:9

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)

@mantepse
Copy link
Collaborator

mantepse commented Mar 8, 2017

comment:10

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.

@mantepse
Copy link
Collaborator

mantepse commented Mar 8, 2017

Changed keywords from none to FriCAS

@rwst
Copy link
Author

rwst commented Mar 8, 2017

comment:12

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.

@mantepse
Copy link
Collaborator

mantepse commented Mar 8, 2017

comment:13

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.

@fchapoton
Copy link
Contributor

comment:14

Here is a tentative of enhancement.


New commits:

7775304trac 22525 better conversion from fricas expressions

@fchapoton
Copy link
Contributor

Branch: public/22525

@fchapoton
Copy link
Contributor

Commit: 7775304

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 12, 2017

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

f9b0635add a test

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 12, 2017

Changed commit from 7775304 to f9b0635

@mantepse
Copy link
Collaborator

comment:17

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.

@mantepse
Copy link
Collaborator

Author: Frédéric Chapoton

@mantepse
Copy link
Collaborator

Reviewer: Martin Rubey

@mantepse
Copy link
Collaborator

comment:19

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

@mantepse
Copy link
Collaborator

comment:20

Looks good!

@vbraun
Copy link
Member

vbraun commented Sep 15, 2017

Changed branch from public/22525 to f9b0635

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

4 participants