Opened 12 years ago

Closed 6 years ago

#10034 closed enhancement (fixed)

Make evaluation possible for 'hold' objects

Reported by: Karl-Dieter Crisman Owned by: Burcin Erocal
Priority: major Milestone: sage-6.10
Component: symbolics Keywords:
Cc: Eviatar Bach, Paul Masson Merged in:
Authors: Eviatar Bach, Ralf Stephan Reviewers: Paul Masson
Report Upstream: N/A Work issues:
Branch: 6e4c716 (Commits, GitHub, GitLab) Commit: 6e4c71610ff7914ccdbeb6ea23825bbd223fce91
Dependencies: Stopgaps:

Status badges

Description

See #9879, where it is now possible to 'hold' a symbolic expression:

sage: a = (pi/12).tan(hold=True)
sage: a
tan(1/12*pi)

However, without going through Maxima and a.simplify(), it isn't clear how to get the actual answer for this. Either by changing simplify() to try simplifying through Pynac first, or by adding something like an a.eval() method, we should make that possible without Maxima.

Attachments (2)

trac_10034.patch (1.7 KB) - added by Eviatar Bach 9 years ago.
trac_10034_2.patch (14.0 KB) - added by Eviatar Bach 9 years ago.

Download all attachments as: .zip

Change History (29)

comment:1 Changed 12 years ago by Karl-Dieter Crisman

This is related to #10071, which is about functions which can't even be evaluated using Maxima or n(). This ticket is saying that one should be able to evaluate all held functions without using Maxima or n(), whether or not a function currently can be evaluated in that way or not.

comment:2 Changed 9 years ago by Eviatar Bach

Cc: Eviatar Bach added

comment:3 Changed 9 years ago by Eviatar Bach

Status: newneeds_review

Here's a patch which makes evaluation possible, simply by walking the expression and evaluating all operations. It does not work for the functions in #10071, because the .operator() method doesn't work on them; I believe this is a separate issue for another ticket though.

Patchbot apply trac_10034.patch

Changed 9 years ago by Eviatar Bach

Attachment: trac_10034.patch added

comment:4 Changed 9 years ago by Karl-Dieter Crisman

Glance looks good. Before I think more about this, a question - is eval the right name for this? I know I'm the one who suggested it... but what do other eval methods do? Also, I think there are a lot of examples which use simplify to evaluate these currently - maybe we could switch them to this (or add this, perhaps). Yes, I agree that #10071 is fine not to try to handle here - that's why I opened #10071.

comment:5 Changed 9 years ago by Eviatar Bach

The _eval_ method for symbolic functions defines what to do when the function is evaluated, and the _eval_self method for expressions tries to do numerical evaluation. Maybe a name like unhold would be better?

Ah right, I'll switch the examples to this. There's no reason why these expressions should be transferred to Maxima simply for evaluating an operation.

comment:6 Changed 9 years ago by Karl-Dieter Crisman

Ok.

As another remark (and you might hate me for this one), I could imagine someone wanting to evaluate some but not all of the held operations. I think this is possible with your patch and judicious use of op and the tree, but at least adding an example of that would be helpful. Particularly in the x * x + x * x example, though, I wonder if it wouldn't be pretty annoying to simplify this to 2 * x * x using this. What do you think about such cases?

comment:7 Changed 9 years ago by Eviatar Bach

I've thought about this. One option is to use pattern matching to specify the parts to evaluate, but I don't how the expression could be reconstructed afterwards.

Another option is to have an argument for providing a list of operators not to evaluate (I think it's better to have an argument to exclude rather than include, because it is difficult to find all the operators in Sage, while finding ones to exclude just involves searching the expression). Then for the 2 * x * x example you could just add operator.mul to the excluded operators and it would work.

comment:8 Changed 9 years ago by Eviatar Bach

Okay, the new patch renames eval to unhold, moves the examples to use the new method, and adds an exclude option. Excluding arithmetic operators doesn't yet work because of #14850.

Patchbot apply trac_10034_2.patch

Last edited 9 years ago by Eviatar Bach (previous) (diff)

Changed 9 years ago by Eviatar Bach

Attachment: trac_10034_2.patch added

comment:9 Changed 9 years ago by Eviatar Bach

Status: needs_reviewneeds_work

Actually, the way I implemented it is not correct, since if the length of the operands is over two it reduces the rest of the operands using the operator. This is the desired behaviour for the arithmetic operators, but not generally.

comment:10 Changed 9 years ago by Jeroen Demeyer

Milestone: sage-5.11sage-5.12

comment:11 Changed 9 years ago by Karl-Dieter Crisman

Maybe this should also fix other places the hold stuff is shown, e.g. functions/trig.py.

comment:12 Changed 9 years ago by For batch modifications

Milestone: sage-6.1sage-6.2

comment:13 Changed 8 years ago by For batch modifications

Milestone: sage-6.2sage-6.3

comment:14 Changed 8 years ago by For batch modifications

Milestone: sage-6.3sage-6.4

comment:15 Changed 8 years ago by Ralf Stephan

Summary: Make evaluation possible for Pynac 'hold' objectssimplify_trig of f(a/b*pi) without Maxima

I don't think an eval member is right here. The end user would expect .simplify_trig() to work, and it does actually. The only problem the original submitter had was the Maxima overhead, so it boils down to a native implementation called from simplify_trig().

comment:16 in reply to:  15 Changed 7 years ago by Ralf Stephan

What I wrote:

I don't think an eval member is right here. The end user would expect .simplify_trig() to work, and it does actually. The only problem the original submitter had was the Maxima overhead, so it boils down to a native implementation called from simplify_trig().

is of course nonsense. Every function that sends the held expression through Maxima unchanged would work because the hold status gets lost. The expansion happens in Pynac's (function)::eval so is already implemented outside Maxima.

comment:17 Changed 7 years ago by Ralf Stephan

Summary: simplify_trig of f(a/b*pi) without MaximaMake evaluation possible for Pynac 'hold' objects

Previous title restored.

comment:18 Changed 7 years ago by Ralf Stephan

Branch: u/rws/make_evaluation_possible_for_pynac__hold__objects

comment:19 Changed 7 years ago by Ralf Stephan

Authors: Eviatar Bach, Ralf Stephan
Commit: 026ab3d85d69ccfb756302593d566f31f013e8c6
Milestone: sage-6.4sage-6.10
Status: needs_workneeds_review
Summary: Make evaluation possible for Pynac 'hold' objectsMake evaluation possible for 'hold' objects

I used the convenient ExpressionTreeWalker that takes care of the caveats mentioned above.


New commits:

026ab3d10034: implement Expression.unhold()

comment:20 Changed 6 years ago by Ralf Stephan

Branch: u/rws/make_evaluation_possible_for_pynac__hold__objectsu/rws/10034-1

comment:21 Changed 6 years ago by git

Commit: 026ab3d85d69ccfb756302593d566f31f013e8c6d414b5de538f2a05d561d428702ae7ab88ca42ff

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

d414b5d10034: increase coverage

comment:22 Changed 6 years ago by Paul Masson

Everything I tested functions just fine, including building documents. Ready for positive review.

One minor quibble: the exclude option references Trac #14850, which is a closed duplicate of #10169. Shouldn't that be updated?

comment:23 Changed 6 years ago by Paul Masson

Cc: Paul Masson added

comment:24 Changed 6 years ago by git

Commit: d414b5de538f2a05d561d428702ae7ab88ca42ff6e4c71610ff7914ccdbeb6ea23825bbd223fce91

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

7b90350Merge branch 'develop' into t/10034/10034-1
6e4c716minor cosmetics

comment:25 Changed 6 years ago by Ralf Stephan

Right. Thanks for the review. Please add your name to Reviewers: too.

comment:26 Changed 6 years ago by Paul Masson

Reviewers: Paul Masson
Status: needs_reviewpositive_review

comment:27 Changed 6 years ago by Volker Braun

Branch: u/rws/10034-16e4c71610ff7914ccdbeb6ea23825bbd223fce91
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.