Opened 4 years ago
Last modified 4 years ago
#24500 closed enhancement
Implement actions using _act_ instead of _call_ — at Version 11
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.5 |
Component: | coercion | Keywords: | |
Cc: | vdelecroix, tscrim | Merged in: | |
Authors: | Jeroen Demeyer | Reviewers: | |
Report Upstream: | N/A | Work issues: | |
Branch: | u/jdemeyer/optimize_action___call__ (Commits, GitHub, GitLab) | Commit: | f2b676ef4e7b13eb4e14f117d39afb8b06ebdc03 |
Dependencies: | #5574 | Stopgaps: |
Description (last modified by )
We change the internal API of Action
: instead of the existing _call_
method, we implement an _act_
method which always takes (g, x)
as arguments, where g
is the acting element and x
is the acted-on element.
The current implementation using _call_
takes a left and right argument. This is less efficient because many actions can be left or right actions. This means that every _call_
method needs to "decode" its arguments as g
and x
.
Work on #24247 shows that a significant amount of time is spent in Action.__call__
. This ticket optimizes that quite a bit. It also adds documentation to Action
.
Timings (compared to before #24247 and #5574):
Note: "before" timings are without #24247, #5574 and #24500. "after" timings are with #24247, #5574 and #24500.
GF(2n)ZZ
Before:
sage: k.<a> = GF(4, impl="ntl"); n = 2; timeit('a ^ n', number=100000, repeat=20) 100000 loops, best of 20: 383 ns per loop
After:
sage: k.<a> = GF(4, impl="ntl"); n = 2; timeit('a ^ n', number=100000, repeat=20) 100000 loops, best of 20: 635 ns per loop
RDFZZ
Before:
sage: a = RDF(2); n = ZZ(2); timeit('a ^ n', number=100000, repeat=20) 100000 loops, best of 20: 156 ns per loop
After:
sage: a = RDF(2); n = ZZ(2); timeit('a ^ n', number=100000, repeat=20) 100000 loops, best of 20: 185 ns per loop
idealZZ
Before:
sage: a = ZZ.ideal(2); n = ZZ(2); timeit('a ^ n', number=10000, repeat=20) 10000 loops, best of 20: 13.3 µs per loop
After:
sage: a = ZZ.ideal(2); n = ZZ(2); timeit('a ^ n', number=10000, repeat=20) 10000 loops, best of 20: 14.1 µs per loop
AAQQ
Before:
sage: a = AA(8); n = 1/3; timeit('a ^ n', number=10000, repeat=20) 10000 loops, best of 20: 12.8 µs per loop
After:
sage: a = AA(8); n = 1/3; timeit('a ^ n', number=10000, repeat=20) 10000 loops, best of 20: 16.6 µs per loop
Change History (11)
comment:1 Changed 4 years ago by
- Description modified (diff)
comment:2 Changed 4 years ago by
- Dependencies set to #5574
comment:3 Changed 4 years ago by
- Branch set to u/jdemeyer/optimize_action___call__
comment:4 Changed 4 years ago by
- Commit set to f2b676ef4e7b13eb4e14f117d39afb8b06ebdc03
comment:5 Changed 4 years ago by
- Component changed from performance to coercion
- Description modified (diff)
- Status changed from new to needs_review
- Summary changed from Optimize Action.__call__ to Implement action using _act_ instead of _call_
comment:6 Changed 4 years ago by
- Description modified (diff)
comment:7 Changed 4 years ago by
- Cc vdelecroix added
- Summary changed from Implement action using _act_ instead of _call_ to Implement actions using _act_ instead of _call_
comment:8 Changed 4 years ago by
- Description modified (diff)
comment:9 follow-up: ↓ 10 Changed 4 years ago by
Do you have timings with this ticket (at least, my understanding of the description is the "after" does not include this ticket)?
comment:10 in reply to: ↑ 9 Changed 4 years ago by
Replying to tscrim:
Do you have timings with this ticket (at least, my understanding of the description is the "after" does not include this ticket)?
The "after" does include this ticket.
comment:11 Changed 4 years ago by
- Description modified (diff)
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
Implement actions using _act_ instead of _call_
Don't use Set_PythonType in IntegerAction