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

use Maxima's trigrat() in symbolic simplify #17065

Open
rwst opened this issue Sep 29, 2014 · 14 comments
Open

use Maxima's trigrat() in symbolic simplify #17065

rwst opened this issue Sep 29, 2014 · 14 comments

Comments

@rwst
Copy link

rwst commented Sep 29, 2014

The ask page
http://ask.sagemath.org/question/11365/simplify-trigonometric-expression/
showed that trigrat() is not used. What a waste.

Upstream: Fixed upstream, but not in a stable release.

Component: symbolics

Keywords: maxima, simplification, trigonometric

Author: Ralf Stephan

Branch/Commit: u/rws/use_maxima_s_trigrat___in_symbolic_simplify @ 3db6f89

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

@rwst rwst added this to the sage-6.4 milestone Sep 29, 2014
@rwst
Copy link
Author

rwst commented Sep 30, 2014

@rwst
Copy link
Author

rwst commented Sep 30, 2014

comment:2

Please review.


New commits:

3db6f8917065: do trigrat after trigexpand; doctest

@rwst
Copy link
Author

rwst commented Sep 30, 2014

Author: Ralf Stephan

@rwst
Copy link
Author

rwst commented Sep 30, 2014

Commit: 3db6f89

@nbruin
Copy link
Contributor

nbruin commented Sep 30, 2014

comment:3

Quite some doctest failures on the buildbot. Some of them are probably not due this particular change, but others most probably are. A few are simply better answers now (that's just a matter of changing the expected output), but there are also some that are much worse. So the change is not uniformly an improvement.

ratsimp claims "canonical form", so that's definitely attractive. However, do we know if its rewrites always apply across the whole domain? We'd have to document if it deviates.

@rwst
Copy link
Author

rwst commented Oct 2, 2014

comment:4

Since setting "needs_work" makes the buildbot link disappear, and I'm too stupid to find the results otherwise (I tried), I cannot sort this out without help, sorry.

@kcrisman
Copy link
Member

kcrisman commented Oct 2, 2014

comment:5

This can even lead to disaster, I think:

sage: F = (4*cos(9*pi/180)^2 - 3)*(4*cos(27*pi/180)^2 - 3)
sage: F.maxima_methods().trigrat()
sqrt(5) + 2*cos(7/10*pi) - 2*cos(1/10*pi) - 2*I*sin(3/5*pi) +
2*I*sin(2/5*pi) + 1

I really am not interested in seeing I turn up in such a simplification, regardless of the "correct" answer.

@nbruin
Copy link
Contributor

nbruin commented Oct 3, 2014

comment:6

Replying to @rwst:

Since setting "needs_work" makes the buildbot link disappear, and I'm too stupid to find the results otherwise (I tried), I cannot sort this out without help, sorry.

Oh, that's silly. I'm setting it back to "needs review" to make the report visible for now. Hopefully we can have this fixed.

@rwst
Copy link
Author

rwst commented Oct 3, 2014

comment:7

Thanks, the link is now here: http://build.sagedev.org/trac/builders/trac_builder/builds/1046
so the ticket can be set to the right status.

@rwst
Copy link
Author

rwst commented Oct 4, 2014

comment:8

Replying to @kcrisman:

This can even lead to disaster, I think:

sage: F = (4*cos(9*pi/180)^2 - 3)*(4*cos(27*pi/180)^2 - 3)
sage: F.maxima_methods().trigrat()
sqrt(5) + 2*cos(7/10*pi) - 2*cos(1/10*pi) - 2*I*sin(3/5*pi) +
2*I*sin(2/5*pi) + 1

I really am not interested in seeing I turn up in such a simplification, regardless of the "correct" answer.

While this one could be resolved by adding a trigreduce to the queue:

sage: F = (4*cos(9*pi/180)^2 - 3)*(4*cos(27*pi/180)^2 - 3)
sage: F.maxima_methods().trigrat()
sqrt(5) + 2*cos(7/10*pi) - 2*cos(1/10*pi) - 2*I*sin(3/5*pi) +
2*I*sin(2/5*pi) + 1
sage: _.maxima_methods().trigreduce()
sqrt(5) - 2*cos(3/10*pi) - 2*cos(1/10*pi) + 1

I cannot see how to fix the doctest failure:

File "src/doc/de/thematische_anleitungen/sage_gymnasium.rst", line 394, in doc.de.thematische_anleitungen.sage_gymnasium
Failed example:
    (sin(x + y)/(log(x) + log(y))).simplify_full()
Expected:
    (cos(y)*sin(x) + cos(x)*sin(y))/log(x*y)
Got:
    ((-I*arctan2(0, x) - I*arctan2(0, y))*sin(x + y) + log(abs(x)*abs(y))*sin(x + y))/(arctan2(0, x)^2 + 2*arctan2(0, x)*arctan2(0, y) + arctan2(0, y)^2 + log(abs(x))^2 + 2*log(abs(x))*log(abs(y)) + log(abs(y))^2)

the minimal case being

sage: log(x).maxima_methods().trigrat()
I*arctan2(0, x) + log(abs(x))

@rwst
Copy link
Author

rwst commented Oct 4, 2014

comment:9

On the other hand, replacing trigrat with trigsimp/trigexpand/trigreduce will fail in this case:

sage: sin(1/8*pi)*sin(3/8*pi)*sin(5/8*pi)*sin(7/8*pi)
sin(7/8*pi)*sin(5/8*pi)*sin(3/8*pi)*sin(1/8*pi)
sage: _.maxima_methods().trigreduce()
TypeError                                 Traceback (most recent call last)
/home/ralf/sage/local/lib/python2.7/site-packages/sage/interfaces/interface.pyc in __init__(self, parent, value, is_name, name)
    624                 self._name = parent._create(value, name=name)
    625             except (TypeError, RuntimeError, ValueError) as x:
--> 626                 raise TypeError(x)
    627 
    628     def _latex_(self):

TypeError: ECL says: Unrecognised output from sp1sintcos.

@nbruin
Copy link
Contributor

nbruin commented Oct 5, 2014

comment:10

Replying to @rwst:

On the other hand, replacing trigrat with trigsimp/trigexpand/trigreduce will fail in this case:
[...]

Yes, I noticed that one too. I have confirmed that the same problem arises in Maxima 5.34.1 on SBCL, so it's probably a maxima problem. This is now:
https://sourceforge.net/p/maxima/bugs/2818/

@rwst
Copy link
Author

rwst commented Oct 31, 2014

Upstream: Fixed upstream, but not in a stable release.

@rwst
Copy link
Author

rwst commented Feb 11, 2015

comment:12

While some of the mentioned problems have been fixed in recent Maxima, this no longer works:

sage: F = (4*cos(9*pi/180)^2 - 3)*(4*cos(27*pi/180)^2 - 3)
sage: F.maxima_methods().trigrat()
sqrt(5) + 2*cos(7/10*pi) - 2*cos(1/10*pi) - 2*I*sin(3/5*pi) +
2*I*sin(2/5*pi) + 1
sage: _.maxima_methods().trigreduce()
sqrt(5) - 2*cos(3/10*pi) - 2*cos(1/10*pi) + 1

@mkoeppe mkoeppe removed this from the sage-6.4 milestone Dec 29, 2022
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