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:

Status badges

Description (last modified by jdemeyer)

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 jdemeyer

  • Description modified (diff)

comment:2 Changed 4 years ago by jdemeyer

  • Dependencies set to #5574

comment:3 Changed 4 years ago by jdemeyer

  • Branch set to u/jdemeyer/optimize_action___call__

comment:4 Changed 4 years ago by git

  • Commit set to f2b676ef4e7b13eb4e14f117d39afb8b06ebdc03

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

dd3b5b5Implement actions using _act_ instead of _call_
f2b676eDon't use Set_PythonType in IntegerAction

comment:5 Changed 4 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • 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 jdemeyer

  • Description modified (diff)

comment:7 Changed 4 years ago by jdemeyer

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

  • Description modified (diff)

comment:9 follow-up: Changed 4 years ago by tscrim

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 jdemeyer

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 jdemeyer

  • Description modified (diff)
Note: See TracTickets for help on using tickets.