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: 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 rws)

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 to float, 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)

plot_with_complex_intermediates.patch (5.3 KB) - added by tkluck 7 years ago.
plot_with_complex_intermediates_002.patch (2.5 KB) - added by tkluck 7 years ago.
trac_13355_plot_with_complex_intermediates.patch (7.8 KB) - added by tkluck 6 years ago.

Download all attachments as: .zip

Change History (30)

Changed 7 years ago by tkluck

comment:1 Changed 7 years ago by tkluck

  • Status changed from new to needs_review

comment:2 Changed 7 years ago by tkluck

  • Description modified (diff)

comment:3 Changed 7 years ago by tkluck

  • Status changed from needs_review to needs_work

There's some issues with the patch as it is, please wait with reviewing.

Changed 7 years ago by tkluck

comment:4 Changed 7 years ago by tkluck

  • 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 tkluck

Added an attachment containing the two patches together, with a proper filename containing the trac number.

comment:6 Changed 6 years ago by chapoton

for the bot:

apply trac_13355_plot_with_complex_intermediates.patch

comment:7 Changed 6 years ago by chapoton

  • Status changed from needs_review to needs_work
  • Work issues set to doctest

one failing doctest, see bot report

comment:8 Changed 6 years ago by tkluck

  • 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: Changed 6 years ago by vdelecroix

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 ?

Version 1, edited 6 years ago by vdelecroix (previous) (next) (diff)

comment:10 in reply to: ↑ 9 ; follow-up: Changed 6 years ago by tkluck

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 vdelecroix

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 jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:13 Changed 6 years ago by kcrisman

  • Cc kcrisman added

comment:14 Changed 6 years ago by eviatarbach

  • 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 vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:16 Changed 6 years ago by rws

  • Status changed from needs_review to needs_work
  • Work issues set to compare patch with #15030

comment:17 Changed 5 years ago by kcrisman

Note that #15030 was apparently merged.

Also, anyone know how this impacts 3d plots?

comment:18 Changed 5 years ago by rws

  • Milestone changed from sage-6.2 to sage-duplicate/invalid/wontfix
  • Status changed from needs_work to needs_review

comment:19 Changed 5 years ago by kcrisman

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 rws

  • Branch set to u/rws/plot_fails_if_a_function_implicitly_needs_complex_intermediate_values

comment:21 Changed 5 years ago by rws

  • Authors set to Timo Kluck
  • 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:

23e82c213355: handle domain properly in evalf functions (doctests)

comment:22 Changed 5 years ago by kcrisman

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 rws

Yes. So change the doctests or abandon them?

comment:24 Changed 5 years ago by kcrisman

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 git

  • Commit changed from 23e82c2ed92225aed5987f3fb22c576860237858 to 65675e8384d489cd8c906cfa56be0c356334320f

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

66aa509Merge branch 'develop' into t/13355/plot_fails_if_a_function_implicitly_needs_complex_intermediate_values
65675e813355 reviewer's patch: fix doctests, typos, PEP8

comment:26 Changed 5 years ago by rws

  • 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 vbraun

  • 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
Note: See TracTickets for help on using tickets.