Opened 10 years ago
Last modified 5 years ago
#7512 needs_info defect
fast_callable should respect the variable order in callable symbolic expressions (treating them like lambda functions rather than like expressions)
Reported by: | jason | Owned by: | was |
---|---|---|---|
Priority: | major | Milestone: | sage-6.4 |
Component: | graphics | Keywords: | |
Cc: | wcauchois, robertwb | Merged in: | |
Authors: | Robert Bradshaw, Tim Dumol, Jason Grout | Reviewers: | Ross Kyprianou |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
This is a somewhat subtle issue if the function is a pure python function (you'd need to analyze the argument names), but maybe a convention is that if variable names are specified, they must match up to argument names in the function, and then they are slotted into the function using a dictionary, so that no matter where in the variable range list the range for x appears, it always is sent to the function as f(x=value).
See http://sagenb.org/home/jason3/302/
I think that each of these should return the same plot:
sage: var('x,y') sage: def f(x,y): ... return x*sin(y) ... sage: plot3d(f, (x,0,3),(y,-6,6),viewer='tachyon') sage: plot3d(f, (y,-6,6),(x,0,3),viewer='tachyon') sage: g(x,y)= x*sin(y) sage: plot3d(g, (x,0,3),(y,-6,6),viewer='tachyon') sage: plot3d(g, (y,-6,6),(x,0,3),viewer='tachyon')
Attachments (1)
Change History (22)
comment:1 Changed 10 years ago by
- Cc wcauchois added
comment:2 Changed 10 years ago by
- Report Upstream set to N/A
Changed 10 years ago by
comment:3 Changed 10 years ago by
- Status changed from new to needs_review
comment:4 Changed 10 years ago by
- Cc robertwb added
We decided that this was just a symptom of a much deeper issue in fast_callable. The patch fixes the issue in fast_callable.
comment:5 Changed 10 years ago by
comment:6 Changed 10 years ago by
- Summary changed from plot3d variable ranges should respect the named variable, if there is one to fast_callable should respect the variable order in callable symbolic expressions (treating them like lambda functions rather than like expressions)
comment:7 Changed 10 years ago by
Applied patch trac-7512-fast_callable-callable_expressions.patch to sage-4.3.2-alpha0 and tried the exact commands below (in that order). I looked for all 4 plots to be the same (as per the ticket description) but only Plot3d No.1 was the same as Plot3d No.3 - AND - Plot3d No.2 was the same as Plot3d No.4. I tried to see if swapping the ranges of x and y, would give me a clue as to what might be going on (refer plot3d No.5 below) but that caused an exception "ZeroDivisionError?: float division" (at line 954: count = (end-start)/step IN sage/misc/misc.pyc)
sage: var('x,y') (x, y) sage: def f(x,y): ....: return x*sin(y) ....: sage: g(x,y)= x*sin(y) sage: sage: plot3d(f, (x,0,3),(y,-6,6),viewer='tachyon') sage: plot3d(f, (y,-6,6),(x,0,3),viewer='tachyon') sage: plot3d(g, (x,0,3),(y,-6,6),viewer='tachyon') sage: plot3d(g, (y,-6,6),(x,0,3),viewer='tachyon') sage: plot3d(f, (x,-6,-6),(y,0,3),viewer='tachyon')
comment:8 Changed 10 years ago by
rossk: we decided that the four plots shouldn't be equal after all; the first should be the same as the third, and the second should be the same as the fourth.
ping to robertwb: if you have time, it'd be great if you could review this too.
comment:9 Changed 10 years ago by
- Reviewers set to Ross Kyprianou
Played with some examples and can see that the examples below act like lambda functions now (regardless of variable name or variable order or function format - all plots came out the same as expected). Hope that helps speed up the review (someone will still have to do the code review)
var('x y v w') def f(x,y): return x*sin(y) g(x,y)=x*sin(y) plot3d(f, (x,0,3), (y,-6,6), viewer='tachyon') plot3d(f, (y,0,3), (x,-6,6), viewer='tachyon') plot3d(f, (v,0,3), (w,-6,6), viewer='tachyon') plot3d(g, (x,0,3), (y,-6,6), viewer='tachyon') plot3d(g, (y,0,3), (x,-6,6), viewer='tachyon') plot3d(g, (v,0,3), (w,-6,6), viewer='tachyon')
comment:10 follow-ups: ↓ 11 ↓ 14 Changed 10 years ago by
I have two dumb questions. Not putting "needs work" in case someone else decides on the review.
- Aren't CallableSymbolicExpressions? already Expressions? How does this get into the else?
if isinstance(x, Expression): et = x vars = et._etb._vars else: from sage.symbolic.callable import is_CallableSymbolicExpression if is_CallableSymbolicExpression(x): vars=x.args()
- Why did you decide again that 1 and 3 should be different from 2 and 4? I'm just curious, since this seems VERY counter-intuitive to me, since we go to the trouble of having g(x=1,y=2)==g(y=2,x=1) for callable guys and not allowing other variable names in them (this came up in the PREP workshop from a user question).
comment:11 in reply to: ↑ 10 ; follow-up: ↓ 12 Changed 10 years ago by
Replying to kcrisman:
- Why did you decide again that 1 and 3 should be different from 2 and 4? I'm just curious, since this seems VERY counter-intuitive to me, since we go to the trouble of having g(x=1,y=2)==g(y=2,x=1) for callable guys and not allowing other variable names in them (this came up in the PREP workshop from a user question).
We do allow other variable names in callable expressions:
sage: f(x,y)=a*x+y sage: f(1,2) a + 2 sage: f(a=2) 2*x + y
I'm also looking at your other points...
comment:12 in reply to: ↑ 11 Changed 10 years ago by
- Why did you decide again that 1 and 3 should be different from 2 and 4? I'm just curious, since this seems VERY counter-intuitive to me, since we go to the trouble of having g(x=1,y=2)==g(y=2,x=1) for callable guys and not allowing other variable names in them (this came up in the PREP workshop from a user question).
We do allow other variable names in callable expressions:
sage: f(x,y)=a*x+y sage: f(1,2) a + 2 sage: f(a=2) 2*x + y
I meant that we aren't allowed to use another variable name to substitute for x and y, but rossk's examples indicate you can do this somehow (?) now since the variables are ignored.
comment:13 Changed 10 years ago by
Replying to kcrisman:
- Why did you decide again that 1 and 3 should be different from 2 and 4? I'm just curious, since this seems VERY counter-intuitive to me, since we go to the trouble of having g(x=1,y=2)==g(y=2,x=1) for callable guys and not allowing other variable names in them (this came up in the PREP workshop from a user question).
I think in retrospect, I agree that this patch does not have the right design decision. Instead of throwing away the var values, we should make the default be the x.args() (so that someone doesn't need to specify the vars keyword if passing in a CallableSymbolicExpression?).
comment:14 in reply to: ↑ 10 Changed 10 years ago by
Replying to kcrisman:
I have two dumb questions. Not putting "needs work" in case someone else decides on the review.
- Aren't CallableSymbolicExpressions? already Expressions? How does this get into the else?
The Expression in the code is different than Symbolic Expressions (instead, they are related to Expression trees, a fast callable construct.
comment:15 Changed 10 years ago by
- Status changed from needs_review to needs_work
comment:16 Changed 10 years ago by
I'm doing quite a bit of work on fast_callable over on #5572. I think I'll try to take care of this issue (with the different design decision I just mentioned) over there.
comment:17 Changed 9 years ago by
- Status changed from needs_work to needs_info
Changing to needs info, pending the decisions and work over on #5572 (i.e., the info needed is: is this ticket obsolete and invalid?)
comment:18 Changed 6 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:19 Changed 6 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:20 Changed 6 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:21 Changed 5 years ago by
- Milestone changed from sage-6.3 to sage-6.4
Robert and I decided that the fourth plot should be the same as the second, but it is not. The error is in the fourth plot. The problem seems to be that in the fourth plot, the g(x,y) is really called as g(x=x_val, y=y_val), but should just be called as g(y_val, x_val) (i.e., not with named variables, but with just positional parameters).