Opened 3 years ago
Last modified 2 years ago
#21274 needs_work enhancement
frac(x) immediate simplifications
Reported by: | rws | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-7.4 |
Component: | symbolics | Keywords: | |
Cc: | mkoeppe | Merged in: | |
Authors: | Ralf Stephan | Reviewers: | |
Report Upstream: | N/A | Work issues: | |
Branch: | u/rws/frac_x__immediate_simplifications (Commits) | Commit: | bf362867b8089a8a68b4e51ac67582cfe63b4679 |
Dependencies: | Stopgaps: |
Description
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)
Change History (11)
comment:1 Changed 3 years ago by
- Branch set to u/rws/frac_x__immediate_simplifications
comment:2 Changed 3 years ago by
- Commit set to bf362867b8089a8a68b4e51ac67582cfe63b4679
- Status changed from new to needs_review
comment:3 Changed 3 years ago by
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
comment:4 Changed 3 years ago by
Actually the mechanics for this exists (eg https://github.com/pynac/pynac/issues/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
.
comment:5 Changed 3 years ago by
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
comment:6 Changed 3 years ago by
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).
comment:7 Changed 3 years ago by
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.
comment:8 Changed 3 years ago by
As a further data point SymPy does not have functions as expression members at all, not even .square()
or .abs()
.
comment:9 follow-up: ↓ 11 Changed 3 years ago by
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).
comment:10 Changed 3 years ago by
- Status changed from needs_review to needs_work
comment:11 in reply to: ↑ 9 Changed 2 years ago by
Replying to vdelecroix:
... 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
.
New commits:
21274: frac(x) immediate simplifications