Skip to content

Restore clock-event enum coverage with older bindings - #2672

Open
rwgk wants to merge 1 commit into
NVIDIA:mainfrom
rwgk:fix_clock_event_coverage_with_older_bindings
Open

Restore clock-event enum coverage with older bindings#2672
rwgk wants to merge 1 commit into
NVIDIA:mainfrom
rwgk:fix_clock_event_coverage_with_older_bindings

Conversation

@rwgk

@rwgk rwgk commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

@mdboom, could you please advise on the intended long-term solution? Given the compatibility matrix we currently run, the narrow dynamic allowance seems appropriate, but I do not want to undo the earlier removal (see History) without understanding its intent.


Summary

Restore a narrowly scoped compatibility allowance in test_enum_coverage.py for ClocksEventReasons.BOARD_LIMIT and ClocksEventReasons.RELIABILITY when the installed cuda.bindings version predates the corresponding NVML enum names.

The allowance is computed dynamically. When the binding names exist, the set is empty and the ordinary full-coverage check still applies. This changes only the coverage test; it does not change runtime behavior.

Why this is needed

This surfaced while validating #2641. That branch exposes the two newer cuda.core clock-event members, while its compatibility matrix also tests against older CUDA 12.9 and 13.3 cuda.bindings packages. All 11 failing Linux ARM64 jobs reached the same enum-coverage case: the wrapper had BOARD_LIMIT and RELIABILITY, but those older bindings did not yet have EVENT_REASON_BOARD_LIMIT and EVENT_REASON_RELIABILITY.

The production mapping intentionally supports this package-version combination by using the stable numeric NVML values when the names are absent. The coverage test's member-count check did not model that exception and therefore rejected a combination that the runtime mapping handles deliberately. Commit df5356d on #2641 restored this dynamic allowance so the older-bindings rows could continue checking every member they actually provide.

History

The history is less straightforward than the resulting change:

  • The same targeted allowance entered public PR 2437 through commit c8c60db.
  • It was then removed it in commit aa10ee9 (Fix enum handling) before PR 2437 was merged.
  • Public PR 2451 subsequently changed the generic coverage test to tolerate the opposite version skew: newer bindings may contain members that an older cuda.core wrapper does not yet expose.
  • The PR 2451 behavior does not cover the case seen in PR 2641, where the wrapper is newer than the installed bindings. The targeted allowance was therefore reintroduced there.

@rwgk rwgk added this to the cuda.bindings 13.4.0 & 12.9.8 milestone Aug 20, 2026
@rwgk rwgk self-assigned this Aug 20, 2026
@rwgk rwgk added P0 High priority - Must do! cuda.core Everything related to the cuda.core module labels Aug 20, 2026
@rwgk
rwgk requested a review from mdboom August 20, 2026 02:58
@github-actions

Copy link
Copy Markdown

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

Labels

cuda.core Everything related to the cuda.core module P0 High priority - Must do!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant