-
Notifications
You must be signed in to change notification settings - Fork 95
Add fast path for indices on non-invertible TransformedGrid #1264
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
base: master
Are you sure you want to change the base?
Add fast path for indices on non-invertible TransformedGrid #1264
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @marcosdanieldasilva !
Added a few suggestions to generalize the code:
- The transform can have multiple steps, so we need to combine all the revertible ones into a new transform in reverse order.
- We can then just apply the created transform to the geometry.
We still need to check if any transform step needs special treatment.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1264 +/- ##
==========================================
- Coverage 87.90% 87.52% -0.38%
==========================================
Files 197 197
Lines 6240 6246 +6
==========================================
- Hits 5485 5467 -18
- Misses 755 779 +24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Júlio Hoffimann <[email protected]>
Co-authored-by: Júlio Hoffimann <[email protected]>
Co-authored-by: Júlio Hoffimann <[email protected]>
Co-authored-by: Júlio Hoffimann <[email protected]>
Co-authored-by: Júlio Hoffimann <[email protected]>
Co-authored-by: Júlio Hoffimann <[email protected]>
Co-authored-by: Júlio Hoffimann <[email protected]>
This PR adds a major performance optimization for finding indices on a
TransformedGrid, specifically when it has a non-invertibleSequentialTransform(like aRotatefollowed by aMorphologicaltransform).Previously, this situation would fall back to the generic
findall(intersects(geometry), domain)method, which is extremely slow because it must perform a costly intersection check for every single cell.This new, specialized method implements a much faster "partial revert" strategy. It introspects the transform sequence, finds the invertible component (using
isinvertible), and reverts the input geometry using only that part. This "un-transforms" the geometry back to the original grid's coordinate space, allowing it to call the existing, highly-optimizedindices(grid.mesh, ...)function on the simple, non-deformed grid. A new test is included to validate this exact scenario.