fix(bindings): CUDA_PYTHON_DISABLE_MAJOR_VERSION_WARNING=0 must not disable it - #2581
Open
LeSingh1 wants to merge 1 commit into
Open
fix(bindings): CUDA_PYTHON_DISABLE_MAJOR_VERSION_WARNING=0 must not disable it#2581LeSingh1 wants to merge 1 commit into
LeSingh1 wants to merge 1 commit into
Conversation
…isable it
The suppression check tests the raw environment string for truthiness:
if os.environ.get("CUDA_PYTHON_DISABLE_MAJOR_VERSION_WARNING"):
return
so `=0` -- the obvious way to spell "no, keep warning me" -- suppresses the
warning just as effectively as `=1`. The warning's own text says
"(Set CUDA_PYTHON_DISABLE_MAJOR_VERSION_WARNING=1 to suppress this warning.)",
which reads as though 0 is the off value, and the other two boolean knobs in
this repository disagree with it:
cuda_bindings/.../_internal/runtime_linux.pyx:29
bool(int(os.getenv('CUDA_PYTHON_CUDA_PER_THREAD_DEFAULT_STREAM', default=0)))
cuda_core/cuda/core/__init__.py:44
if int(os.environ.get("CUDA_CORE_DONT_FIX_TAB_COMPLETION", "0")):
Both parse with int(), so `=0` is off for them. A user who sets all three to
0 gets the behaviour they asked for from two of them and the opposite from
this one, silently losing a compatibility warning that exists to explain why
their driver is too old.
Parse the value with int() here too. Unset and empty still mean "not
disabled". A value that is not an integer keeps the old set-means-disabled
behaviour, so anyone currently relying on a spelling like `=true` does not
start seeing the warning again -- `0` is the only input whose meaning
changes.
Contributor
github-actions
Bot
added
the
cuda.bindings
mdboom
requested changes
Aug 11, 2026
Comment on lines
+16
to
+36
| """Whether the user asked to suppress the major-version warning. | ||
|
|
||
| ``=0`` means "do not suppress". A bare truthiness test on the raw string | ||
| made ``CUDA_PYTHON_DISABLE_MAJOR_VERSION_WARNING=0`` suppress the warning | ||
| -- the exact opposite of what the warning itself tells the user to type, | ||
| and the opposite of the other boolean knobs in this repository | ||
| (``CUDA_PYTHON_CUDA_PER_THREAD_DEFAULT_STREAM`` and | ||
| ``CUDA_CORE_DONT_FIX_TAB_COMPLETION``), which both parse their value with | ||
| ``int()``. | ||
|
|
||
| Unset and empty still mean "not disabled". A value that is not an integer | ||
| keeps the old set-means-disabled behaviour, so anyone currently relying on | ||
| a spelling like ``=true`` does not silently start seeing the warning again. | ||
| """ | ||
| raw = os.environ.get(_DISABLE_WARNING_ENV_VAR, "").strip() | ||
| if not raw: | ||
| return False | ||
| try: | ||
| return int(raw) != 0 | ||
| except ValueError: | ||
| return True |
Contributor
There was a problem hiding this comment.
Make a helper function for parsing a bool-like value from an environment variable. Call it envvar_bool and put it in utils/__init__.py. Then just use that, passing in the environment variable name, from warn_if_cuda_major_version_mismatch.
There are other instances of parsing bool-like envvars throughout the codebase that could then be updated to use this new helper function.
Comment on lines
+87
to
+111
| @pytest.mark.agent_authored(model="claude-opus-5") | ||
| @pytest.mark.parametrize( | ||
| ("raw", "expected"), | ||
| [ | ||
| pytest.param("0", False, id="zero"), | ||
| pytest.param(" 0 ", False, id="zero-padded"), | ||
| pytest.param("", False, id="empty"), | ||
| pytest.param(" ", False, id="blank"), | ||
| pytest.param("1", True, id="one"), | ||
| pytest.param("2", True, id="two"), | ||
| # Not an integer: keep the old set-means-disabled behaviour so no | ||
| # one relying on a spelling like `=true` starts seeing the warning | ||
| # again. | ||
| pytest.param("true", True, id="true"), | ||
| pytest.param("yes", True, id="yes"), | ||
| ], | ||
| ) | ||
| def test_disable_flag_parsing(self, monkeypatch, raw, expected): | ||
| monkeypatch.setenv("CUDA_PYTHON_DISABLE_MAJOR_VERSION_WARNING", raw) | ||
| assert _version_check._warning_disabled() is expected | ||
|
|
||
| @pytest.mark.agent_authored(model="claude-opus-5") | ||
| def test_disable_flag_unset(self, monkeypatch): | ||
| monkeypatch.delenv("CUDA_PYTHON_DISABLE_MAJOR_VERSION_WARNING", raising=False) | ||
| assert _version_check._warning_disabled() is False |
Contributor
There was a problem hiding this comment.
Given my other suggestion of a generic helper function, the test should be also made generic (and not tied to a specific environment variable).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The suppression check tests the raw environment string for truthiness (
_version_check.py:35):so
CUDA_PYTHON_DISABLE_MAJOR_VERSION_WARNING=0— the obvious way to spell "no, keep warning me" — suppresses the warning exactly as effectively as=1.Two things make that a trap rather than a convention:
(Set CUDA_PYTHON_DISABLE_MAJOR_VERSION_WARNING=1 to suppress this warning.), which implies0is the off value.int():A user (or a CI job template, or a Dockerfile) that sets all three to
0gets the behaviour they asked for from two of them and the opposite from this one — silently losing a compatibility warning whose whole purpose is to explain why their driver is too old for the wheel they installed.Fix
Parse the value with
int()here too, in a small_warning_disabled()helper.Compatibility: unset and empty still mean "not disabled", every non-zero integer still disables, and a value that is not an integer keeps the old set-means-disabled behaviour so nobody relying on a spelling like
=truestarts seeing the warning again.0is the only input whose meaning changes — which is the point.Also uses the env-var name constant in the warning text so the message and the lookup cannot drift apart.
Tests
Added to
cuda_bindings/tests/test_version_check.py:test_disable_flag_parsing— parametrised over0,0, empty, blank,1,2,true,yes.test_disable_flag_unset.test_warning_not_suppressed_when_env_var_is_zero— end-to-end throughwarn_if_cuda_major_version_mismatch, mirroring the existingtest_warning_suppressed_by_env_varit sits next to.The existing
test_warning_suppressed_by_env_var(=1) is unchanged and still passes.What I ran
Environment: macOS, no CUDA driver and no CUDA toolkit, so
cuda.bindingsis not built here.cuda_bindings/tests/test_version_check.pyitself — it importscuda.bindings.driverat module level._version_check.pyimports onlyos/threading/warningsat module level and pulls incuda.bindings.driverinside the function, so I loaded the real module with a stubbed driver (CUDA_VERSION=13000,cuDriverGetVersion -> 12080) and counted emitted warnings for each env value:python -m py_compile,ruff check,ruff format --checkon both changed files — clean, no new findings against amainbaseline.warn_if_cuda_major_version_mismatchhas exactly one non-test caller,Device_ensure_cuda_initializedincuda_core/cuda/core/_device.pyx:1613-1625, which calls it immediately after a successfulcuInit(0)and does not inspect the env var itself.