Skip to content

Truncate nested inputs longer than the fitted width - #1263

Open
solarsys wants to merge 1 commit into
sunlabuiuc:masterfrom
solarsys:fix/nested-processors-truncate
Open

solarsys wants to merge 1 commit into
sunlabuiuc:masterfrom
solarsys:fix/nested-processors-truncate

Conversation

@solarsys

@solarsys solarsys commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

Problem

NestedSequenceProcessor, NestedFloatsProcessor, DeepNestedSequenceProcessor and DeepNestedFloatsProcessor pad each visit to the longest visit seen in fit(), plus padding. They never truncate a longer one.

When processors are fitted on a training split, a validation or test visit can be longer than any training visit. That sample then has a different width from the others, and batching fails, because collate_fn_dict_with_padding pads only the first dimension. The deep processors fail even earlier, while building the sample.

On master: fit on visits of 2 codes, process a visit of 4, and you get width 4. Stacking it with a normal sample raises.

Change

  • A visit longer than the fitted width is truncated to it, keeping the first codes/values. Each processor logs one warning that names the counts and points to padding as the way to keep more.
  • The existing padding argument still widens the fitted size, so longer visits fit without truncation.
  • The shared warning helper lives in base_processor.py. It tolerates processors pickled before this change.

Deliberately unchanged: visits per group in the deep processors

The deep processors also pad the visits-per-group dimension to the size seen in fit(), and longer groups would break batching in the same way. However, existing tests (test_deep_nested_sequence_processors.py, e.g. test_all_none_values) process groups with more visits than were fitted and expect them to be kept. I left that behaviour alone rather than change those expectations. Happy to truncate that dimension too in a follow-up if that's the intended contract.

Tests, docs, example

  • New tests/core/test_nested_processor_truncation.py covers:

    • all four processors, including both forward_fill settings;
    • stacking a longer and a shorter sample;
    • exactly one warning per processor;
    • padding keeping longer visits.

    It fails on master and passes here.

  • Existing nested and deep-nested processor tests pass (60 in total).

  • The NestedSequenceProcessor docs page gets an "Output width" section, and the docstrings now describe truncation.

  • New examples/nested_sequence_fit_on_train.py: fit on training samples, apply to a test patient with a longer visit, and show the padding alternative.

  • Full core suite: Ran 1388 tests … OK (skipped=76). tools/check_pr_rules.py passes.

Reported by a downstream EHR project that hit expected sequence of length … errors the first time it fitted processors on the training split only.

🤖 Generated with Claude Code

NestedSequenceProcessor, NestedFloatsProcessor, DeepNestedSequenceProcessor
and DeepNestedFloatsProcessor pad each visit to the longest visit seen in
fit() (plus `padding`), but never truncated a longer one. When processors
are fitted on a training split, a validation/test visit can be longer, so
samples got different widths and batching failed (the default collate
pads only the first dimension); the deep processors failed already while
building the sample.

Longer visits are now truncated to the fitted width, keeping the first
codes/values, and each processor logs one warning that names the counts
and points to `padding`. The visits-per-group dimension of the deep
processors is unchanged: existing tests expect it to grow past the fitted
size.

- base_processor: shared one-time warning helper.
- tests/core/test_nested_processor_truncation.py: all four processors,
  stacking, one warning per processor, and `padding` keeping more.
- docs: "Output width" section on the NestedSequenceProcessor page.
- examples/nested_sequence_fit_on_train.py: fit on train, apply to a
  test patient with a longer visit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant