Skip to content

Retry tests failing on cuBLAS allocation errors the rerun filter missed #6916

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.

Sign up for GitHub

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

Merged
albertvillanova merged 1 commit into from
Aug 28, 2026
Merged

Retry tests failing on cuBLAS allocation errors the rerun filter missed #6916

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@ check_dirs := examples tests trl

ACCELERATE_CONFIG_PATH = `pwd`/examples/accelerate_configs

# Transient infrastructure errors that are worth retrying, matched against "<ExceptionType>: <message>":
# - OSError, Timeout, HTTPError 502/504: Hub flakiness
# - OutOfMemoryError, STATUS_ALLOC_FAILED: GPU memory pressure from the parallel workers
rerun_errors := (OSError|Timeout|HTTPError.*502|HTTPError.*504|OutOfMemoryError|STATUS_ALLOC_FAILED)

@cursor cursor Bot Aug 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Incomplete GPU retry patterns

Medium Severity

rerun_errors adds STATUS_ALLOC_FAILED but still omits the torch.testing.assert_close wrapper text. OOMs raised inside assert_close surface as RuntimeError: Comparing... with OutOfMemoryError only on __cause__, which --only-rerun never inspects, so that failure shape still skips retries and can fail a healthy branch under GPU memory pressure.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7ce6dae. Configure here.

@albertvillanova albertvillanova Aug 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The analysis is accurate, but this is a deliberate scope boundary rather than an oversight, and adding the wrapper text would make the filter worse rather than better.

The gap is real and described in the PR body under "Not addressed here", and it is the half of #6917 that is intentionally left open. The reason it is not closed here is that the only available handle is the wrapper text itself, and that text says nothing about memory pressure:

raise RuntimeError(
    f"Comparing\n\n"
    f"{pair}\n\n"
    f"resulted in the unexpected exception above. "
    ...
) from error

The message carries only the tensor pair repr and boilerplate. torch.testing emits it for any exception it does not expect during a comparison, so matching it would also retry genuine defects, a dtype bug surfacing as a TypeError, an unsupported layout raising NotImplementedError, and so on. A deterministic bug would still fail all five attempts, so it would not be hidden, but a nondeterministic one would be silently retried into green, which is precisely what a narrow --only-rerun exists to prevent. Trading a false negative on OOM for a false positive on real bugs is the wrong direction.

An earlier revision of this PR did include that pattern and it was removed for this reason.

I verified the resulting behaviour against synthetic failures for each shape. Retried: a directly raised OutOfMemoryError, and CUBLAS_STATUS_ALLOC_FAILED. Not retried: an OOM wrapped by assert_close, a plain failing assertion, an ordinary assert_close value mismatch, and a TypeError raised inside a comparison.

The correct place to fix the remaining shape is pytest-rerunfailures, which matches only the outermost exception:

def _try_match_error(rerun_errors, excinfo):
    if excinfo:
        err = f"{excinfo.type.__name__}: {excinfo.value}"
        for rerun_regex in rerun_errors:
            if re.search(rerun_regex, err):
                return True
    return False

If that walked __cause__ and __context__, the existing OutOfMemoryError pattern would match the wrapped case on its own, with no ambiguity and no need for a wrapper-text heuristic. #6917 stays open to track that.


test:
pytest -n auto -m "not slow and not low_priority" -s -v --reruns 5 --reruns-delay 1 --only-rerun '(OSError|Timeout|HTTPError.*502|HTTPError.*504|OutOfMemoryError)' tests
pytest -n auto -m "not slow and not low_priority" -s -v --reruns 5 --reruns-delay 1 --only-rerun '$(rerun_errors)' tests

precommit:
python scripts/add_copyrights.py
Expand Down
Loading