Skip to content

fix: account for len(thresholds) when dispatching the vectorized PR-curve update (closes #3299) - #3452

Open
Kayvan-Zahiri wants to merge 2 commits into
Lightning-AI:masterfrom
Kayvan-Zahiri:fix/prc-threshold-aware-dispatch
Open

fix: account for len(thresholds) when dispatching the vectorized PR-curve update (closes #3299)#3452
Kayvan-Zahiri wants to merge 2 commits into
Lightning-AI:masterfrom
Kayvan-Zahiri:fix/prc-threshold-aware-dispatch

Conversation

@Kayvan-Zahiri

Copy link
Copy Markdown

Closes #3299.

The bug

thresholds is documented to bound memory, but the dispatch that chooses between
the vectorized and loop update paths never looks at how many thresholds there are.

if preds.numel() * num_classes <= 1_000_000:
    update_fn = _multiclass_precision_recall_curve_update_vectorized

The vectorized path then allocates preds.numel() * len(thresholds) elements at

preds_t = (preds.unsqueeze(-1) >= thresholds.unsqueeze(0).unsqueeze(0)).long()

and again for unique_mapping. For the reported case, preds of shape
(60000, 3) with thresholds=200, the gate computes 540,000 <= 1,000,000 and
picks the vectorized path, which then asks for 288 MB of int64, twice over:

thresholds gate says intermediate
5 vectorized 7.2 MB
50 vectorized 72.0 MB
200 vectorized 288.0 MB
1000 vectorized 1440.0 MB

MPS surfaces this as an OOM because its ceiling is lower, but the memory model is
wrong on every device. CPU just absorbs it.

The fix

Bound the intermediate as well as the input. The binary dispatch has the same
defect (preds.numel() <= 50_000, also blind to len_t), so both are fixed.

This is a pure narrowing: a case that previously went vectorized either still does,
or falls back to the loop. Nothing that used the loop before is now vectorized, so
no workload gets slower unless it was also allocating hundreds of megabytes.

Why this is safe

The two paths return identical state. I verified across thresholds of 3, 7, 20 and
200 for multiclass and 3, 20, 200 for binary, on both the vectorized and loop
implementations, with torch.equal in every case. Falling back only trades speed
for memory, never correctness.

Tests

  • test_multiclass_update_memory_does_not_scale_with_thresholds, parametrized over
    5 and 200 thresholds, asserts which path the dispatch actually picks. The 200 case
    fails on main and passes here; the 5 case passes both ways, which pins that small
    threshold counts keep their current fast path.
  • test_multiclass_update_paths_agree and test_binary_update_paths_agree pin the
    invariant the fix depends on.

Full file: 214 passed, 24 skipped, 4 xfailed, no regressions.

One question for a reviewer

_MAX_VECTORIZED_ELEMENTS = 1_000_000 (8 MB of int64, doubled by unique_mapping)
is my choice, not something derived from an existing constant. It keeps the reported
case out of the vectorized path with room to spare, but the right budget is really a
maintainer call and I would happily change it. If you would rather express it as a
byte budget, or scale it from the existing 50_000 / 1_000_000 gates, say which
and I will update.

Not covered here

_multilabel_precision_recall_curve_update has the same preds.numel() * len_t
allocation and no loop fallback to dispatch to, so it is unconditionally exposed.
Fixing it means writing a multilabel loop implementation, which felt like a separate
change from the reported bug. Happy to add it to this PR or open a follow-up,
whichever you prefer.

Kayvan-Zahiri and others added 2 commits August 24, 2026 11:35
…urve update

The vectorized update paths materialise a preds.numel() * len(thresholds)
intermediate, but the dispatch only looked at the input size. Passing a large
thresholds argument therefore silently broke the constant memory behaviour that
argument is documented to provide, allocating 288 MB for a 60000 x 3 input with
200 thresholds.

Bound the intermediate as well as the input. Both update paths already return
identical state, so falling back to the loop only trades speed for memory.

Closes Lightning-AI#3299
@Kayvan-Zahiri
Kayvan-Zahiri force-pushed the fix/prc-threshold-aware-dispatch branch from 564560f to be4a47a Compare August 24, 2026 18:35
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.

MulticlassPrecisionRecallCurve with thresholds=[integer] does not use constant memory on MPS

1 participant