Opened 10 years ago

Last modified 16 months ago

#5572 needs_work defect

fast_callable improvements (followup for #5093)

Reported by: robertwb Owned by: cwitty
Priority: major Milestone: sage-6.4
Component: basic arithmetic Keywords:
Cc: robertwb, timdumol, eviatarbach, novoselt Merged in:
Authors: Jason Grout Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jason)

The code at #5093 is very good and ready to go in, but there are several improvements that have been suggested and agreed work on at a later date. They are posted here so we can merge and close the other ticket.

Specifically, this ticket addresses these issues:

  • fast_callable on list,tuple,vector,matrix arguments
  • fast_callable on constant arguments
  • fast_callable of a zero multivariate polynomial returns a zero of the base ring, without paying attention to the types of the arguments
  • in general replaces calls to fast_float with calls to fast_callable.
  • Carries out the deprecation in #5413 (removes the functionality deprecated there)

Because of some of the far-reaching changes, this should probably not be merged in a point-point release.

What is not fixed:

  • Robert's suggestion: an option that uses a fast domain, if it's there, but ignores the domain parameter if it's not (I don't mind the idea, and the implementation is easy; what should the syntax be? Part of my problem picking a syntax is that I don't want to promise that a specialized interpreter is always faster than the Python-object interpreter, so I don't particularly want to use the word "fast" in any option names.)

The work on this ticket:

  • Makes #8450 easy, probably
  • Makes #7512 invalid, probably

See also

  • #10087
  • #7165
  • #11766 -- rewrite some of fast_callable to have its own recursion stack

Attachments (7)

improve_fast_callable.patch (77.5 KB) - added by jason 9 years ago.
fast_callable_complex.patch (4.4 KB) - added by jason 9 years ago.
apply on top of previous patch
improve_fast_callable.2.patch (77.1 KB) - added by jason 9 years ago.
rebase to 4.4.1
improve_fast_callable.3.patch (74.8 KB) - added by jason 9 years ago.
apply instead of previous patches
improve_fast_callable.4.patch (75.1 KB) - added by jason 9 years ago.
apply instead of previous patches (now doctests in plot/*.py[x] pass)
improve_fast_callable.5.patch (82.6 KB) - added by jason 9 years ago.
apply instead of previous patches (now almost all doctests in plot/plot3d/ pass)
improve_fast_callable.6.patch (87.5 KB) - added by jason 9 years ago.
apply instead of previous patches (fixed a bunch of stuff so even more doctests pass)

Download all attachments as: .zip

Change History (42)

comment:1 Changed 10 years ago by cwitty

  • Owner changed from somebody to cwitty
  • Status changed from new to assigned

comment:2 follow-up: Changed 10 years ago by jason

More fast_callable improvements:

  • domain=float is the same as domain=RDF, but domain=complex is not the same as domain=CDF. Make domain=complex use the same interpreter as domain=CDF
  • if g is a callable expression, then fast_callable(g) should use g.args() for the variables, not g.variables(). Hmm...or maybe return an error if g.args() is not equal to g.variables(), since every variable really does have to be satisfied.

comment:3 Changed 10 years ago by robertwb

Both good points. At least g.variables() should be a subset of g.args().

comment:4 in reply to: ↑ 2 Changed 9 years ago by jason

  • Report Upstream set to N/A

Replying to jason:

  • if g is a callable expression, then fast_callable(g) should use g.args() for the variables, not g.variables(). Hmm...or maybe return an error if g.args() is not equal to g.variables(), since every variable really does have to be satisfied.

#7512 may take care of this.

Changed 9 years ago by jason

comment:5 Changed 9 years ago by jason

This is a big patch to fast_callable which

  • makes it work for lists/tuples like fast_float does
  • adds lots of _fast_callable_ methods to make it work on a lot of different constant objects (integers/rationals/RDF/RR/CDF/CC)
  • refactors the code a bit
  • in general replaces calls to fast_float with calls to fast_callable.

The patch also breaks some things---it's still a work in progress.

Changed 9 years ago by jason

apply on top of previous patch

comment:6 Changed 9 years ago by jason

The second patch is a broken attempt at streamlining the complex support since Cython now supports C99 complex numbers.

comment:7 Changed 9 years ago by jason

  • Status changed from new to needs_work

comment:8 Changed 9 years ago by jason

  • Cc robertwb timdumol added

CCing: robertwb since fast_float was his idea originally

timdumol since he's expressed interest in working on this sort of thing.

The two patches need work at this point. In particular, the improvements patch needs docs added/changed, and ptestlong needs to be run to check for breakage.

The complex number patch needs an overhaul, as I think it's completely broken at this point. The complex number patch is not necessary for resolving the issues on this ticket.

comment:9 Changed 9 years ago by jason

#8450 would be a good ticket to (trivially) fix after this ticket moves plotting solely over to fast_callable.

Changed 9 years ago by jason

rebase to 4.4.1

comment:10 Changed 9 years ago by jason

I think it might be best just to fix #7512 in this ticket.

Changed 9 years ago by jason

apply instead of previous patches

comment:11 Changed 9 years ago by jason

Still not done. A clean design still needs to happen for fast_callable on symbolics without specified variables, and the nvars option seems like a hack to make plotting work with lambda functions (since we now match the argument names of lambda functions by default).

comment:12 Changed 9 years ago by jason

Also, something should be done to put fast_float back (and its file) as a convenience method.

Changed 9 years ago by jason

apply instead of previous patches (now doctests in plot/*.py[x] pass)

Changed 9 years ago by jason

apply instead of previous patches (now almost all doctests in plot/plot3d/ pass)

comment:13 Changed 9 years ago by jason

I had to rework some of the transformation code because the returned function often did not have the same keyword arguments as the original function, which throws off plotting.

Changed 9 years ago by jason

apply instead of previous patches (fixed a bunch of stuff so even more doctests pass)

comment:14 Changed 9 years ago by jason

To delete the fast_eval.so file from the build directory (necessary so that the cython fast_eval is eliminated when testing), do:

cd $SAGE_ROOT/devel/sage/build
find . -name fast_eval.so | xargs rm

comment:15 Changed 9 years ago by jason

Progress report: my current patch queue has the following failures on make ptestlong on Sage 4.5.2:

	sage -t  -long 4.5.2/devel/sage/sage/structure/sage_object.pyx # 1 doctests failed
	sage -t  -long 4.5.2/devel/sage/sage/ext/fast_callable.pyx # Exception from doctest framework
	sage -t  -long 4.5.2/devel/sage/sage/rings/polynomial/polynomial_element.pyx # 9 doctests failed
	sage -t  -long 4.5.2/devel/sage/sage/stats/hmm/distributions.pyx # 1 doctests failed

comment:16 Changed 9 years ago by jason

(my patch queue is up at http://sage.math.washington.edu/home/jason/sage-4.5.2-patches and this ticket involves patches up to pickling.patch)

comment:17 Changed 9 years ago by jason

  • Description modified (diff)

comment:18 Changed 9 years ago by jason

  • Authors set to Jason Grout
  • Description modified (diff)

comment:19 Changed 9 years ago by jason

  • Description modified (diff)

comment:20 Changed 8 years ago by kcrisman

What is the status of switching to fast_callable? There seem to be a number of tickets which would benefit, not to mention the fact that fast_float has old-style documentation which looks bad in the command line :) Plus, if fast_float is to be deprecated (even though it seems to use fast_callable under the hood) it would be helpful to start that process.

Anyway, just curious.

comment:21 Changed 8 years ago by jason

A long time ago I worked on the patches you see here; I believe that all of my work is here, anyway. I probably won't have time to work on this this summer, due to notebook enhancements. I'd like to make this one of the projects for next summer if someone hasn't beaten me to it by then.

comment:22 Changed 8 years ago by jason

The problem is that I think my patch is too big and needs to be broken down into smaller patches that change less at each step. It's been a long time, but I think I'm remembering that right. Anyways, as noted above, my patch queue is up on sage.math and anyone is welcome to work on it.

comment:23 Changed 7 years ago by jason

  • Description modified (diff)

comment:24 Changed 6 years ago by eviatarbach

  • Cc eviatarbach added

comment:25 Changed 6 years ago by eviatarbach

Can I just create a new patch to switch plotting to use fast_callable? Using fast_float causes big problems for plotting, namely that any point at which the function is complex at some value in the call stack fails to plot. For example, the plot of abs(log(x)) will not show up for negative x, despite the output being guaranteed to be real, since fast_float will evaluate log(x) first and choke on the complex number.

comment:26 Changed 6 years ago by jason

Sounds fine to me. Especially if all the doctests pass (including the new one or two that you write :).

comment:27 Changed 6 years ago by eviatarbach

Okay, thanks. This is now #15030, with a patch up.

comment:28 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:29 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:30 Changed 5 years ago by kcrisman

Note that #15030 is now merged. How that does affect whatever else needs to happen here?

comment:31 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:32 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:33 Changed 4 years ago by novoselt

  • Cc novoselt added

comment:34 Changed 16 months ago by rws

Anyone interested in the deprecation of fast_float can find relevant tickets at https://trac.sagemath.org/wiki/symbolics#fast_floatdeprecation

comment:35 Changed 16 months ago by rws

Without looking in detail I'd guess that implementing https://github.com/pynac/pynac/issues/8 will make ex.subs() a much faster alternative to fast_callable.

Note: See TracTickets for help on using tickets.