Opened 9 years ago
Closed 9 years ago
#7661 closed defect (fixed)
maxima interface gives precedence to function dictionary instead of local variables
Reported by: | burcin | Owned by: | burcin |
---|---|---|---|
Priority: | critical | Milestone: | sage-4.4 |
Component: | interfaces | Keywords: | maxima |
Cc: | robert.marik | Merged in: | sage-4.4.alpha0 |
Authors: | Burcin Erocal | Reviewers: | Robert Mařík |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
From the sage-devel thread:
http://groups.google.com/group/sage-devel/t/c89582242c83a349
On Fri, 11 Dec 2009 13:46:31 +0100 Nathann Cohen <nathann.cohen@gmail.com> wrote: > sage: var('delta k') > sage: m1=2*delta**2 + 2**2*delta*k > sage: n=delta*k+2 > sage: m2=(2*delta)**2+(k-1)*4 > sage: m=(delta+delta*k-(delta-1)) > sage: ((m1/n)-(m2/n)).expand().simplify()
On 4.3.rc0, I get this:
TypeError: unsupported operand parent(s) for '*': 'Symbolic Ring' and '<class 'sage.functions.generalized.FunctionDiracDelta'>'
The Maxima interface seems to give precedence to the global function dictionary instead of the local variables when converting Maxima output back to Sage expressions.
sage: dirac_delta(x) dirac_delta(x) sage: maxima(dirac_delta(x)) delta(x)
Attachments (2)
Change History (16)
comment:1 Changed 9 years ago by
comment:2 Changed 9 years ago by
sage: d = var('delta') sage: e = d._maxima_() sage: sage.calculus.calculus.symbolic_expression_from_maxima_element(e) dirac_delta
somewhere in symbolic_expression_from_maxima_element(), the string from maxima is looked up in sage.calculus.calculus._syms, which by default has 'delta': dirac_delta . So this is what's happening, next, SR() barfs on trying to turn dirac_delta into a symbolic expression, at which point people who just wanted their variable 'delta' back get confused and frustrated.
sage: del sage.calculus.calculus._symsdelta? sage: sage.calculus.calculus.symbolic_expression_from_maxima_element(e) delta
That may not be such a good idea, however, since what sage calls dirac_delta, maxima refers to as delta. Nevertheless, since reset('delta') appears to remove delta from that dictionary, perhaps var('delta') should also do so?
Of course, what happens when someone does a Laplace transform with delta as a sage variable will then come out confusing and wrong. At least the current behavior is merely broken.
comment:3 Changed 9 years ago by
- Priority changed from major to critical
- Status changed from new to needs_review
attachment:trac_7661-maxima_convert_back.patch fixes the problem reported above, and a similar problem with function conversions back from maxima reported in comment:2:ticket:8459.
Changed 9 years ago by
comment:4 Changed 9 years ago by
I updated attachment:trac_7661-maxima_convert_back.patch to remove a doctest fix broken by a previous patch in my queue (#6949, symbol...
line in sage/symbolic/ring.pyx).
This patch depends on #7748.
comment:5 Changed 9 years ago by
Thanks for working onthis. Is #7748 the only prerequisity? I installed three patches as described at #7748 and got the following error
patching file sage/calculus/calculus.py Hunk #3 succeeded at 1414 with fuzz 1 (offset -4 lines). Hunk #5 FAILED at 1455 1 out of 14 hunks FAILED -- saving rejects to file sage/calculus/calculus.py.rej abort: patch failed to apply
comment:6 Changed 9 years ago by
- Owner changed from was to burcin
comment:7 Changed 9 years ago by
Hello Burcin
I think that two lines should be removed from the patch
global _syms _syms = sage.symbolic.pynac.symbol_table.get('maxima', {})
I updated your patch, it is now http://user.mendelu.cz/marik/sage/trac_7661-maxima_convert_back2.patch
If everything will work, I'll return in few (several) hours with positive review (tests in functions, interfaces, symbolics and calculus passed, now running all the test).
Robert
comment:8 Changed 9 years ago by
OK. That is one approach to solving this problem. Now we need to rebase the patch at #8237 so that it applies on top of these. Removing the offending hunk from calculus.py
should be enough for that.
Note that your updated patch shows you as the author. I'd appreciate it if you changed that back.
Thanks.
comment:9 Changed 9 years ago by
- Status changed from needs_review to needs_work
Sure, it was intended as temporary patch and from this reason I did not upload to trac server unless tested. I got some doctest failures in three files. See http://boxen.math.washington.edu/home/marik/ and the files a, b and c.
I think that b is simple to fix, but have no idea about a and c.
comment:10 Changed 9 years ago by
- Cc robert.marik added
comment:11 Changed 9 years ago by
Thanks a lot for the quick feedback.
a
is because you have the pynac package from #8644 installed, but not the corresponding patch from #8565.b
is a simple import problem, fixed by the updated attachment:trac_7661-maxima_convert_back.take2.patch- I can't reproduce
c
here. It doesn't seem to be related to the changes in ticket. Do you have any other patches applied?
comment:12 Changed 9 years ago by
- Status changed from needs_work to needs_review
I tested it on a fresh install and seems that all a,b,c are resolved. I am running all tests again, to be sure :)
comment:13 Changed 9 years ago by
- Reviewers set to Robert Mařík
- Status changed from needs_review to positive_review
Tests passed, postive review, thanks for fixing - very very usefull ticket.
Release manager: apply only trac_7661-maxima_convert_back.take2.patch
comment:14 Changed 9 years ago by
- Merged in set to sage-4.4.alpha0
- Resolution set to fixed
- Status changed from positive_review to closed
Merged "trac_7661-maxima_convert_back.take2.patch" in 4.4.alpha0
People run into this all the time, evidently: