Opened 7 years ago
Closed 5 years ago
#13355 closed defect (fixed)
doctest proper handling of domain in evalf functions
Reported by:  tkluck  Owned by:  burcin 

Priority:  major  Milestone:  sage6.3 
Component:  symbolics  Keywords:  evalf, fast_callable 
Cc:  kcrisman, eviatarbach  Merged in:  
Authors:  Timo Kluck  Reviewers:  Ralf Stephan 
Report Upstream:  N/A  Work issues:  
Branch:  65675e8 (Commits)  Commit:  65675e8384d489cd8c906cfa56be0c356334320f 
Dependencies:  Stopgaps: 
Description (last modified by )
Previous description of ticket:
Plotting currently fails when plotting a symbolic function that evaluates to complex values:
sage: def evalf_func(self,x,parent=None): ....: return parent(I*x) if parent!=None else I*x ....: sage: f = function('f', evalf_func=evalf_func) sage: plot(abs(f(x)),0,5) verbose 0 (2392: plot.py, generate_plot_points) WARNING: When plotting, failed to evaluate function at 199 points. verbose 0 (2392: plot.py, generate_plot_points) Last error message: 'unable to simplify to float approximation'
This is because plot
uses fast_float
, and fast_float
cannot deal with the intermediate complex value.
The attached patch first changes these things:
 it adds a method to
class Function
that allows one to see what output range is to be expected for a given input range. By default, it will return a complex field.  it overrides this method for builtin functions, which return floats for float input
 this is then used by
fast_float
to throw an exception if complex values are to be expected
This exception will cause plot
to fall back to fast_callable
instead of fast_float
. However, fast_callable
needs patching too:
 it now uses the
_evalf_
method when available instead of just__call__
. Otherwise,plot
will try to coerce the result tofloat
, which also cannot deal with complex intermediate values.
An alternative to this step would be to fix float
coercion, which I haven't looked into.
Attachments (3)
Change History (30)
Changed 7 years ago by
comment:1 Changed 7 years ago by
 Status changed from new to needs_review
comment:2 Changed 7 years ago by
 Description modified (diff)
comment:3 Changed 7 years ago by
 Status changed from needs_review to needs_work
Changed 7 years ago by
comment:4 Changed 7 years ago by
 Status changed from needs_work to needs_review
The _002.patch solves the issues I mentioned. Setting status back to "needs review".
comment:5 Changed 7 years ago by
Added an attachment containing the two patches together, with a proper filename containing the trac number.
comment:6 Changed 6 years ago by
for the bot:
apply trac_13355_plot_with_complex_intermediates.patch
comment:7 Changed 6 years ago by
 Status changed from needs_review to needs_work
 Work issues set to doctest
one failing doctest, see bot report
Changed 6 years ago by
comment:8 Changed 6 years ago by
 Keywords needsreview removed
 Status changed from needs_work to needs_review
 Work issues doctest deleted
This should fix the doctest problem.
Apply trac_13355_plot_with_complex_intermediates.patch
comment:9 followup: ↓ 10 Changed 6 years ago by
Hello,
I don't understand your example. You define f
to take complex values, how do you want to plot it as a real function? Could you be more explicit about the problem you intend to solve ? What kind of picture you would like to obtain with such input?
Note that for that purpose, there is yet complex_plot
{{{ sage: def evalf_func(self,x,parent=None): ....: return parent(I*x) if parent!=None else I*x ....: sage: f = function('f', evalf_func=evalf_func) sage: complex_plot(f, (1,1), (1,1)).show() }}}
comment:10 in reply to: ↑ 9 ; followup: ↓ 11 Changed 6 years ago by
Thanks for looking at this!
Replying to vdelecroix:
I don't understand your example. You define
f
to take complex values, how do you want to plot it as a real function?
My example was
sage: plot(abs(f(x)),0,5)
Note the abs
.
Now the point is that
sage: abs(f(3)) abs(f(3)) sage: float(abs(f(3))) 3.0
both work, but plotting doesn't:
plot(abs(f(x)),x,0,10) verbose 0 (2430: plot.py, generate_plot_points) WARNING: When plotting, failed to evaluate function at 199 points. verbose 0 (2430: plot.py, generate_plot_points) Last error message: 'unable to simplify to float approximation'
The reason is that there is an optimization to use fast_callable
for calculating plot points. The current implementation assumes that all intermediate values are floats, whereas we can still make a real plot when the argument to abs is complex. This patch should fix that.
Is this any clearer? It would be great if you could test/review the patch!
comment:11 in reply to: ↑ 10 Changed 6 years ago by
Replying to tkluck:
Is this any clearer?
I see. Sorry for my too fast lecture and thanks for the clarification!
It would be great if you could test/review the patch!
It was my intention ;)
comment:12 Changed 6 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:13 Changed 6 years ago by
 Cc kcrisman added
comment:14 Changed 6 years ago by
 Cc eviatarbach added
#15030 fixes the same issue in a different (and I think simpler) way. I tried your example and it works with the #15030 patch as well.
I also think this is a significant issue; you can't even plot functions like abs(log(x))
properly. tkluck, it would be great if you could look at my patch and if you think it's a better solution review it.
comment:15 Changed 6 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:16 Changed 6 years ago by
 Status changed from needs_review to needs_work
 Work issues set to compare patch with #15030
comment:17 Changed 6 years ago by
Note that #15030 was apparently merged.
Also, anyone know how this impacts 3d plots?
comment:18 Changed 6 years ago by
 Milestone changed from sage6.2 to sageduplicate/invalid/wontfix
 Status changed from needs_work to needs_review
comment:19 Changed 6 years ago by
One thing that could be done here is if there are any significant/useful examples on this ticket that weren't added in #15030, they could be added here...
comment:20 Changed 5 years ago by
 Branch set to u/rws/plot_fails_if_a_function_implicitly_needs_complex_intermediate_values
comment:21 Changed 5 years ago by
 Commit set to 23e82c2ed92225aed5987f3fb22c576860237858
 Description modified (diff)
 Keywords evalf fast_callable added
 Milestone changed from sageduplicate/invalid/wontfix to sage6.3
 Reviewers set to Ralf Stephan
 Status changed from needs_review to needs_work
 Summary changed from Plot fails if a function implicitly needs complex intermediate values to doctest proper handling of domain in evalf functions
 Work issues compare patch with #15030 deleted
I have extracted the doctests from the patch and changed the description accordingly. However, some doctests fail.
sage t long src/sage/ext/fast_callable.pyx ********************************************************************** File "src/sage/ext/fast_callable.pyx", line 397, in sage.ext.fast_callable.? Failed example: fc(3,4) Expected: 12 Got: f(3, 4) ********************************************************************** File "src/sage/ext/fast_callable.pyx", line 405, in sage.ext.fast_callable.? Failed example: fc(3,4) Expected: 12*I Got: g(3, 4) ********************************************************************** 1 item had failures: 2 of 47 in sage.ext.fast_callable.? [586 tests, 2 failures, 3.54 s]  sage t long src/sage/ext/fast_callable.pyx # 2 doctests failed 
New commits:
23e82c2  13355: handle domain properly in evalf functions (doctests)

comment:22 Changed 5 years ago by
Hmm, these doctests look strange to me anyway.
parent!=None
should be
parent is not None
I think. The failures are actually probably features, not bugs, since f
and g
are symbolic functions, which are now not automatically evaluated, if that makes sense...
comment:23 Changed 5 years ago by
Yes. So change the doctests or abandon them?
comment:24 Changed 5 years ago by
If you agree that this is proper behavior, then we might as well change them. Also, the plot one is definitely useful.
comment:25 Changed 5 years ago by
 Commit changed from 23e82c2ed92225aed5987f3fb22c576860237858 to 65675e8384d489cd8c906cfa56be0c356334320f
comment:26 Changed 5 years ago by
 Status changed from needs_work to positive_review
But then, somehow we want 12
or 12*I
as a result too, but for this the function needs a simplify()
member like hypergeometric
(#2516) has.
comment:27 Changed 5 years ago by
 Branch changed from u/rws/plot_fails_if_a_function_implicitly_needs_complex_intermediate_values to 65675e8384d489cd8c906cfa56be0c356334320f
 Resolution set to fixed
 Status changed from positive_review to closed
There's some issues with the patch as it is, please wait with reviewing.