Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fast_callable improvements (followup for #5093) #5572

Open
robertwb opened this issue Mar 19, 2009 · 35 comments
Open

fast_callable improvements (followup for #5093) #5572

robertwb opened this issue Mar 19, 2009 · 35 comments

Comments

@robertwb
Copy link
Contributor

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:

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:

See also

CC: @robertwb @TimDumol @eviatarbach @novoselt @orlitzky

Component: basic arithmetic

Author: Jason Grout

Issue created by migration from https://trac.sagemath.org/ticket/5572

@robertwb robertwb added this to the sage-5.11 milestone Mar 19, 2009
@sagetrac-cwitty sagetrac-cwitty mannequin self-assigned this Mar 26, 2009
@jasongrout
Copy link
Member

comment:2

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.

@robertwb
Copy link
Contributor Author

comment:3

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

@jasongrout
Copy link
Member

comment:4

Replying to @jasongrout:

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

@jasongrout
Copy link
Member

comment:5

Attachment: improve_fast_callable.patch.gz

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.

@jasongrout
Copy link
Member

Attachment: fast_callable_complex.patch.gz

apply on top of previous patch

@jasongrout
Copy link
Member

comment:6

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

@jasongrout
Copy link
Member

comment:8

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.

@jasongrout
Copy link
Member

comment:9

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

@jasongrout
Copy link
Member

rebase to 4.4.1

@jasongrout
Copy link
Member

comment:10

Attachment: improve_fast_callable.2.patch.gz

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

@jasongrout
Copy link
Member

Attachment: improve_fast_callable.3.patch.gz

apply instead of previous patches

@jasongrout
Copy link
Member

comment:11

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).

@jasongrout
Copy link
Member

comment:12

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

@jasongrout
Copy link
Member

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

@jasongrout
Copy link
Member

Attachment: improve_fast_callable.4.patch.gz

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

@jasongrout
Copy link
Member

comment:13

Attachment: improve_fast_callable.5.patch.gz

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.

@jasongrout
Copy link
Member

Attachment: improve_fast_callable.6.patch.gz

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

@jasongrout
Copy link
Member

comment:14

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

@jasongrout
Copy link
Member

comment:15

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

@jasongrout
Copy link
Member

comment:16

(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)

@jasongrout

This comment has been minimized.

@jasongrout

This comment has been minimized.

@jasongrout
Copy link
Member

Author: Jason Grout

@jasongrout

This comment has been minimized.

@kcrisman
Copy link
Member

comment:20

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.

@jasongrout
Copy link
Member

comment:21

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.

@jasongrout
Copy link
Member

comment:22

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.

@jasongrout

This comment has been minimized.

@eviatarbach
Copy link

comment:25

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.

@jasongrout
Copy link
Member

comment:26

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

@eviatarbach
Copy link

comment:27

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

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@kcrisman
Copy link
Member

comment:30

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

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@rwst
Copy link

rwst commented Jan 14, 2018

comment:34

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

@rwst
Copy link

rwst commented Jan 14, 2018

comment:35

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

@DaveWitteMorris

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants