Opened 7 years ago
Closed 6 years ago
#13355 closed defect (fixed)
doctest proper handling of domain in evalf functions
Reported by: | tkluck | Owned by: | burcin |
---|---|---|---|
Priority: | major | Milestone: | sage-6.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 7 years ago by
for the bot:
apply trac_13355_plot_with_complex_intermediates.patch
comment:7 Changed 7 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 needs-review 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 follow-up: ↓ 10 Changed 6 years ago by
Hello,
I don't understand your example. You did not even defined f
, how do you want to plot it? It has nothing to do with 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: float(f(3)) Traceback (most recent call last) ... TypeError: unable to simplify to float approximation
Could you be more explicit about the problem you intend to solve ?
comment:10 in reply to: ↑ 9 ; follow-up: ↓ 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 sage-5.11 to sage-5.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 sage-6.1 to sage-6.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 sage-6.2 to sage-duplicate/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 6 years ago by
- Branch set to u/rws/plot_fails_if_a_function_implicitly_needs_complex_intermediate_values
comment:21 Changed 6 years ago by
- Commit set to 23e82c2ed92225aed5987f3fb22c576860237858
- Description modified (diff)
- Keywords evalf fast_callable added
- Milestone changed from sage-duplicate/invalid/wontfix to sage-6.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 6 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 6 years ago by
Yes. So change the doctests or abandon them?
comment:24 Changed 6 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 6 years ago by
- Commit changed from 23e82c2ed92225aed5987f3fb22c576860237858 to 65675e8384d489cd8c906cfa56be0c356334320f
comment:26 Changed 6 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 6 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.