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

frac(x) immediate simplifications #21274

Open
rwst opened this issue Aug 18, 2016 · 12 comments
Open

frac(x) immediate simplifications #21274

rwst opened this issue Aug 18, 2016 · 12 comments

Comments

@rwst
Copy link

rwst commented Aug 18, 2016

As a followup to #21232 these automatic simplifications should be added:

sage: frac(frac(x))
frac(x)
sage: frac(floor(x))  # ceil
0
sage: assume(x, 'integer')
sage: frac(x)
0
sage: assume(x > 0)
sage: frac(tanh(x))
tanh(x)

CC: @mkoeppe

Component: symbolics

Author: Ralf Stephan

Branch/Commit: u/rws/frac_x__immediate_simplifications @ bf36286

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

@rwst rwst added this to the sage-7.4 milestone Aug 18, 2016
@rwst
Copy link
Author

rwst commented Sep 5, 2016

@rwst
Copy link
Author

rwst commented Sep 5, 2016

Author: Ralf Stephan

@rwst
Copy link
Author

rwst commented Sep 5, 2016

New commits:

bf3628621274: frac(x) immediate simplifications

@rwst
Copy link
Author

rwst commented Sep 5, 2016

Commit: bf36286

@mkoeppe
Copy link
Member

mkoeppe commented Sep 6, 2016

comment:3

This all seems to work, but it's handling very specific cases and it would be much nicer if the system knew this:

sage: floor(x).is_integer()
True     # wishful

@rwst
Copy link
Author

rwst commented Sep 7, 2016

comment:4

Actually the mechanics for this exists (eg pynac/pynac#78) but floor, as it is not a GinacFunction, needs a different approach in Pynac than the ones implemented in that issue. Best I think is to first review #12121 and then after merge make floor a GinacFunction.

@tscrim
Copy link
Collaborator

tscrim commented Sep 7, 2016

comment:5

A very micro optimization:

sage: %timeit ZZ.zero()
The slowest run took 46.49 times longer than the fastest. This could mean that an intermediate result is being cached.
10000000 loops, best of 3: 61.5 ns per loop
sage: %timeit 0    # Preparsed as Integer(0) with the 0 being a python int
The slowest run took 40.29 times longer than the fastest. This could mean that an intermediate result is being cached.
10000000 loops, best of 3: 71 ns per loop
sage: z = int(0)
sage: %timeit Integer(z)
The slowest run took 53.93 times longer than the fastest. This could mean that an intermediate result is being cached.
10000000 loops, best of 3: 75.2 ns per loop

@videlec
Copy link
Contributor

videlec commented Sep 19, 2016

comment:6

Why is this part of the function frac? The function should not care about simplifying the result in any way. If you want a custom frac you would better consider having a frac method to symbolic expression (and having the function frac calling it).

@rwst
Copy link
Author

rwst commented Sep 19, 2016

comment:7

Very interesting. I completely forgot about these. First, if you look at functions/* you'll note a lot of expression arguments handled by custom code in eval() member functions, so called special values. We should write a Expression member function for all these functions? That would be an even larger class than it is now.

Secondly, I removed the Expression.sin() member and the only doctest that fails is the one explicitly calling it. So, it seems to me, other than taste or maybe convenience, there is no reason to have it. Didn't Occam demand not to multiply symbols beyond necessity?

I'm aware of the function eval() member being too general for expressions, and I'm constantly moving code into Pynac, so the special handling in eval() should be seen as a first step for testing code that is well maintainable because it's Python. If you insist I'll immediately move the frac code to C++. But please don't enlarge Expression any further and help with efforts to actually make it smaller.

@rwst
Copy link
Author

rwst commented Sep 19, 2016

comment:8

As a further data point SymPy does not have functions as expression members at all, not even .square() or .abs().

@videlec
Copy link
Contributor

videlec commented Dec 24, 2016

comment:9

The main difference with SymPy is that Sage deals with a lot of different objects. sqrt can be called on integers, rationals, polynomials, power series, ... Not only symbolic expression. You really want to minimize type checking in the function to minimize the overhead (see e.g. the kind of discussions we had in #11332).

@rwst
Copy link
Author

rwst commented Dec 14, 2017

comment:11

Replying to @videlec:

... You really want to minimize type checking in the function to minimize the overhead (see e.g. the kind of discussions we had in #11332).

More and more I agree with this, see #24178. Still, I really don't want the symbolic version as Expression member but as standalone symbolic function, i.e., in sage/functions.

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

4 participants