Skip to content

Store exact history per batch element - #1502

Open
fepegar wants to merge 11 commits into
fepegar/batch-redesign-corefrom
fepegar/batch-redesign-history
Open

Store exact history per batch element#1502
fepegar wants to merge 11 commits into
fepegar/batch-redesign-corefrom
fepegar/batch-redesign-history

Conversation

@fepegar

@fepegar fepegar commented Jul 20, 2026

Copy link
Copy Markdown
Member

[Generated by a coding agent]


What

Replace shared-plus-optional history with one exact transform history per batch element.

Why

The old model persisted _batch_size, _batched_keys, and _keep inside public transform params, then reconstructed histories during unbatching. That representation required special cases in composition, adapters, inversion, replay, and batch containers.

How

  • add a reusable exact-history mixin for both batch types
  • split transient batched params into clean per-element traces when recording
  • preserve histories automatically through factories and unbatching
  • remove _per_element_history, history slicing, manual adoption, and OneOf/SomeOf freezing
  • keep a vectorized inverse when histories match and use per-element inversion when they diverge

Validation

  • full test suite
  • Ruff format/lint and ty
  • documentation tests and build
  • full prek hook suite, including Xenon

Replacement stack

  1. #1500 — padding type prerequisite
  2. #1501 — batch factories, prototypes, and schema
  3. This PR — exact per-element batch history
  4. #1503 — mapping, replay, and adapters
  5. #1504 — documentation and benchmarks

Copilot AI review requested due to automatic review settings July 20, 2026 21:06
@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

📖 Docs Preview

Preview of the documentation for this PR:

🔗 https://smokeshow.helpmanual.io/5l3l2x5m68393s5h320l/

Built from 6c3ee6d

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors TorchIO’s batch transform history to store an exact, per-element history list (instead of shared history plus per-element slicing), and updates transform wrapping/unwrapping and inversion behavior to work with divergent element histories.

Changes:

  • Introduces _BatchedHistoryMixin to manage exact per-element histories for ImagesBatch and SubjectsBatch, including divergent-history inversion behavior.
  • Updates Transform.forward() to eagerly materialize clean per-element history traces (dropping internal batching bookkeeping keys from public history).
  • Updates tests and migration docs to use histories / history(index) and new divergent-history semantics (e.g., get_inverse_transform() raising on divergence).

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_swap.py Update per-instance assertions to read per-element histories.
tests/test_spike.py Update per-instance assertions to read per-element histories.
tests/test_spatial.py Update per-instance spatial assertions to read per-element histories.
tests/test_per_instance.py Update per-instance history expectations and out-of-range behavior.
tests/test_one_of.py Update OneOf divergent history expectations and clearing behavior.
tests/test_normalize.py Update per-instance normalize assertions to read per-element histories.
tests/test_noise.py Update per-instance noise assertions to read per-element histories.
tests/test_motion.py Update per-instance motion assertions to read per-element histories.
tests/test_labels_to_image.py Update per-instance label-stat assertions to use history(index).
tests/test_ghosting.py Update per-instance ghosting assertions to read per-element histories.
tests/test_gamma.py Update per-instance gamma assertions to read per-element histories.
tests/test_flip.py Update per-instance flip assertions to read per-element histories.
tests/test_blur.py Update per-instance blur assertions to read per-element histories.
tests/test_bias_field.py Update per-instance bias field assertions to read per-element histories.
tests/test_batch.py Remove manual history adoption; rely on automatic preservation via per-element histories.
tests/test_anisotropy.py Update per-instance anisotropy assertions to read per-element histories.
tests/conftest.py Update vectorization test helper to replay params from per-element history.
src/torchio/transforms/transform.py Build and append per-element history traces; propagate histories through wrap/unwrap paths.
src/torchio/transforms/monai_adapter.py Remove manual history adoption after restacking subjects.
src/torchio/transforms/inverse.py Delegate inverse application to batch types for correct divergent-history handling.
src/torchio/transforms/cornucopia_adapter.py Remove manual history adoption after restacking subjects.
src/torchio/transforms/compose.py Simplify per-element apply/rebatch logic; rely on automatic history preservation.
src/torchio/data/batch.py Move batch containers to _BatchedHistoryMixin; remove legacy per-element history machinery.
src/torchio/data/batch_history.py New mixin implementing exact per-element history storage and divergent-history inversion behavior.
docs/get-started/migration.md Update migration guidance: restacking preserves exact per-element histories automatically.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/conftest.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

Comment thread src/torchio/data/batch_history.py
@fepegar
fepegar force-pushed the fepegar/batch-redesign-history branch from f992093 to c49206f Compare July 20, 2026 21:33
@fepegar
fepegar requested a review from Copilot July 20, 2026 21:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Comment thread src/torchio/transforms/transform.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Comment thread src/torchio/data/batch_history.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

@fepegar
fepegar force-pushed the fepegar/batch-redesign-history branch from a2250fc to 61c539c Compare July 20, 2026 22:06
@fepegar
fepegar requested a review from Copilot July 20, 2026 22:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 3 comments.

Comment thread src/torchio/data/batch_history.py
Comment thread tests/test_batch.py Outdated
Comment thread tests/test_batch.py
@fepegar
fepegar force-pushed the fepegar/batch-redesign-history branch from 61c539c to fa4ffb3 Compare July 20, 2026 22:18
@fepegar
fepegar requested a review from Copilot July 20, 2026 22:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

fepegar added 9 commits July 20, 2026 23:54
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: baf5f11a-f67c-44dc-804e-6be849fa9160
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: baf5f11a-f67c-44dc-804e-6be849fa9160
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: baf5f11a-f67c-44dc-804e-6be849fa9160
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: baf5f11a-f67c-44dc-804e-6be849fa9160
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: baf5f11a-f67c-44dc-804e-6be849fa9160
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: baf5f11a-f67c-44dc-804e-6be849fa9160
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: baf5f11a-f67c-44dc-804e-6be849fa9160
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: baf5f11a-f67c-44dc-804e-6be849fa9160
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: baf5f11a-f67c-44dc-804e-6be849fa9160

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

@fepegar
fepegar force-pushed the fepegar/batch-redesign-history branch from 9823221 to 80f7b1a Compare July 20, 2026 22:58
@fepegar
fepegar requested a review from Copilot July 20, 2026 22:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.

Comment thread src/torchio/transforms/compose.py Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: baf5f11a-f67c-44dc-804e-6be849fa9160

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: baf5f11a-f67c-44dc-804e-6be849fa9160

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/torchio/transforms/transform.py:278

  • _build_history_traces() deep-copies the full params dict once per batch element when _batched_keys is absent. For large (but batch-shared) parameter dicts this scales CPU+memory with batch_size even though every element’s trace is identical. You can deep-copy params once and reuse the same copied dict across the per-element AppliedTransform objects (the objects can remain distinct to satisfy the independent-trace invariant).
        batched_keys = params.get("_batched_keys")
        if batched_keys is None:
            return [
                self._make_applied_transform(_copy.deepcopy(params))
                for _ in range(batch_size)
            ]

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants