Skip to content

pyop3: Cache AxisForest.getitem - #5562

Open
pbrubeck wants to merge 1 commit into
mainfrom
pbrubeck/pyop3-axis-forest-getitem-cache
Open

pbrubeck wants to merge 1 commit into
mainfrom
pbrubeck/pyop3-axis-forest-getitem-cache

Conversation

@pbrubeck

@pbrubeck pbrubeck commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

AxisForest.getitem was trying the indices on each of its trees and skipping the trees that they did not match. The trees were caching only successful indexing, so every indexing of a forest was indexing the mismatched trees again. In Firedrake this was happening on every Function.assign or zero on a subset, e.g. each DirichletBC.zero.

AxisForest.getitem is now cached for static indices, with the same strategy as AbstractNonUnitAxisTree.getitem:

 AxisForest.getitem(indices)
+  if indices have loop indices
+    return _getitem_uncached(indices)
+  return _getitem_cached(indices)          # @cached_method()

+AxisForest._getitem_uncached(indices)
   for tree in trees
     try tree.getitem(indices)
     except InvalidIndexTargetException, IncompatibleAxisTargetException: skip

Indexing with loop indices is still not cached, so the FIXME about this bottleneck now names loop indices.

Evidence

test_axis_forest_getitem_is_cached (new) indexes a forest twice, and fails if the second indexing indexes any of its trees:

  • Before: fails
    After: passes

DirichletBC.zero for CG2 on a 16×16 mesh (minimum of 200 calls):

before after pyop2
bc.zero(Function) 0.53 ms 0.065 ms 0.006 ms
bc.zero(Cofunction) 0.53 ms 0.069 ms 0.006 ms

Merge Danger

Door: two-way

Blast Radius: small

Only indexing of axis forests with static indices changes. The indexed trees are now kept for the lifetime of the forest, as they already were for the lifetime of each tree.


AI assistance: this PR was written with Claude Code (Claude Opus 5.5).

🤖 Generated with Claude Code

@pbrubeck pbrubeck added the LLM used An LLM was used in the production of this PR label Oct 26, 2026
@pbrubeck
pbrubeck force-pushed the pbrubeck/pyop3-axis-forest-getitem-cache branch 2 times, most recently from a6abbac to 71c229e Compare October 26, 2026 16:55
@pbrubeck pbrubeck changed the title pyop3: Cache failed indexing of axis trees pyop3: Cache AxisForest.getitem Oct 26, 2026
AxisForest.getitem tries the indices on each of its trees and skips the
trees that they do not match. The trees cache only successful indexing, so
every indexing of a forest indexed the mismatched trees again. In Firedrake
this happens on every Function.assign or zero on a subset, e.g. each
DirichletBC.zero.

Cache AxisForest.getitem for static indices, with the same strategy as
AbstractNonUnitAxisTree.getitem.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pbrubeck
pbrubeck force-pushed the pbrubeck/pyop3-axis-forest-getitem-cache branch from 71c229e to ef7f902 Compare October 26, 2026 17:10
@pbrubeck
pbrubeck added this pull request to stack #5563 October 26, 2026 17:13
@pbrubeck pbrubeck added the pyop3 label Oct 26, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LLM used An LLM was used in the production of this PR pyop3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant