Optimize DecohesionTransform using F.conv1d (#201)#1072
Open
JGiraldo29 wants to merge 2 commits into
Open
Conversation
|
@JGiraldo29 is attempting to deploy a commit to the scroll Team on Vercel. A member of the Team first needs to authorize it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Performance fix for the
DecohesionTransformintroduced in #201. It uses the same math and has the same output while being faster on both CPU and GPU.What changed
In
batchgeneratorsv2/transforms/spatial/decohesion.pythe Pythonforloop in_causal_smearhas been replaced with a singleF.conv1dcall after permuting the smear axis to the innermost position.How it works
The original _causal_smear uses a Python loop to accumulate shifted slices. This is mathematically a causal 1-D convolution, but written as a loop it incurs one Python iteration and one kernel dispatch per tap.
This PR replaces the loop with a single F.conv1d call:
This permutation step is required due to how cuDNN handles 3D convolutions. The backend is highly sensitive to kernel orientation:
Why it's useful
The changes implemented here aim to speed up training when decohesion is in the augmentation pipeline. The reduction is large on GPU (where the original loop incurs
K-1kernel launches per call) and significant on modern CPUs.Evidence
Equivalence
Bit-equivalent to the original within float32 summation roundoff. Tested across 22 synthetic cases (11 CPU + 11 CUDA) covering 2D/3D shapes, every smear axis, kernel widths from 1 to 16, and channel counts from 1 to 4. Max absolute error across all cases: 5e-7 (float32 epsilon).
Verified on real scroll data: a (160, 160, 160) crop from the PHerc172 OME-Zarr (
53keV_7.91um.zarr, scale 0) producesmax|old - new| = 1.19e-07.The existing
__main__self-test indecohesion.pystill passes all five structural assertions (identity at p=0, segmentation untouched, one-sided trailing-vs-leading smear, density modulation, finite output) with identical numerical output to the original.Speed (CUDA — Tesla T4)
Speed (CPU — Intel Core Ultra 7 255H)
Decohesion old vs new
Decohesion old minus new
Details
taxiscontract as the original (params['axis'] + 1, indexing into a(C, *spatial)tensor).(C, X, Y)and 3D(C, X, Y, Z)tensors.K=1short-circuit preserved.How to reproduce
Two scripts are included:
The second script streams a small crop from
dl.ash2txt.organd generates the figures attached to this PR.test_causal_smear_rewrite.py
validate_on_real_data.py
Scope
This PR only touches
_causal_smear. The Python-loop accumulators in_density_weightand the avg-pool path are already vectorized; no change needed there. Subsequent PRs can address other per-transform optimizations (e.g. the device-sync.item()calls inSqueezeTransform.get_parameters).Addresses the optimization side of the Decohesion item of #201.
The use of LLMs was limited for REVIEWING and TESTING, as well as assisting in some parts of the changes. No agentic coding was used for this.