Repository navigation
Stop gb.ss import from turning the SuiteSparse JIT on - #630
Merged
Merged
Conversation
eriknw
force-pushed
the
40-ss-import-jit-control
branch
4 times, most recently
from
August 27, 2026 16:46
e3f45f4 to
6ed0f67
Compare
eriknw
force-pushed
the
40-ss-import-jit-control
branch
from
September 18, 2026 03:25
6ed0f67 to
f9d2799
Compare
eriknw
force-pushed
the
40-ss-import-jit-control
branch
from
September 18, 2026 03:46
f9d2799 to
854fce4
Compare
eriknw
changed the base branch from
39-repr-chop-complex-overflow
to
09-array-udt-udf-views
September 23, 2026 18:18
eriknw
force-pushed
the
40-ss-import-jit-control
branch
from
September 23, 2026 18:19
854fce4 to
2082505
Compare
eriknw
force-pushed
the
40-ss-import-jit-control
branch
from
September 24, 2026 20:06
2082505 to
4c75928
Compare
Importing ``graphblas.ss`` raised ``jit_c_control`` from SuiteSparse's default ``'run'`` to ``'on'``. That submodule is reached by attribute access, so ``print(gb.ss.about["library_version"])``, the line a user writes for a bug report, silently changed the numerical machinery of every later call in the process. ``'on'`` is also what makes SuiteSparse load kernels from its on-disk cache, and that cache is keyed on the version triple alone. Two builds sharing a triple but differing in a JIT-visible header therefore share kernels: locally, kernels compiled by a 10.4.0 dated "July 31, 2026" were loaded by a 10.4.0 dated "Aug 7, 2026", whose GB_Matrix_content.h had moved a field. A four-element complement-masked assign returned GrB_OUT_OF_MEMORY, and a full suite run showed 25 unrelated-looking failures. Reported upstream with a standalone C reproducer. The import now repairs the conda-baked compiler path and nothing else. The bump to ``'on'`` moves to ``_enable_jit_for_udt``, called when a UDT registers a type or arms an op with C source: acts that plainly involve compiling C, so enabling the compiler is not a surprise. An explicit ``'off'`` or ``'pause'`` is honored; only SuiteSparse's own non-compiling defaults are raised. ``fix_jit_config()`` is unchanged, since enabling compilation is what its caller asked for. Two details the hooks make necessary: ``_probe_jit`` registers a UDT to verify the compiler, and registering a UDT now asks to enable the JIT, which would probe again. The inner probe registered the probe type and the outer one then died redeclaring it through cffi, so a working environment got ``False`` from the first ``fix_jit_config()`` and ``True`` from every call after. ``_probing_jit`` makes the re-entrant request answer yes without starting a second probe. ``_maybe_warn_no_jit`` blamed ``jit_c_control`` for a UDT that is not expressible as a C struct, and now names the one cause its call site can produce. It used to re-derive the cause from the live config, which is not the state codegen read. Codegen reads the dtype; it never looks at the control or the compiler path, so neither can be why it declined. On a host whose JIT probe fails the control reads back ``'run'`` and ``_enable_jit_for_udt`` has cached its ``False``, which is enough for the old message to blame the control again. That is the ubuntu-latest and macos-latest conda runners, so it was a failing test as well as a message that sent the reader to a setting which cannot fix it. Dropping the environment branches costs no diagnostic: a UDT that does have a C form is armed and never reaches this warning, so an unusable compiler was already silent here. ``_enable_jit_for_udt`` caches whether this process can compile, which is settled once, but it was also returning before re-raising the control, which is separate state that anything may have moved since. Restoring a saved ``'run'`` after an earlier op had enabled the JIT therefore disabled compilation for the rest of the process, silently: the next op is armed with C source SuiteSparse will never build, and nothing warns, because codegen succeeded. The cached path now raises ``'run'`` and ``'load'`` the same way the uncached one does, and still honors an explicit ``'off'`` or ``'pause'``. Before this commit the question could not arise: the import forced ``'on'``, so a saved value was always ``'on'`` too. Its test drives a stand-in config so the assertion holds on every host, and pins the live round trip separately: a library built without the JIT clamps every write down to ``'run'``, and would never read back the ``'on'`` the test is about. One prose correction while in the file: ``_probe_jit`` claimed SuiteSparse flips the control to ``'load'`` after a failed compile, as though that were the only outcome. ``git grep -n "GB_jit_control = "`` in SuiteSparse finds eight places that lower it. ``GB_jitifyer_set_control`` is the one that comes first: it takes the minimum of the requested value and ``'run'`` on a library built without the JIT, so ``'on'`` cannot be set there however good the compiler is. On the load path, a compile failure gives ``'load'``, a load failure ``'run'``, and a failed hash insert ``'pause'``. The docstring says the value is not ours to predict, so the contract here is the bool. The new subprocess tests read the control through the C API rather than ``gb.ss.config``, because reaching for ``gb.ss`` is the thing under test.
The "JIT compiler auto-fix" section still said the import bumps jit_c_control from 'run' to 'on'. After the previous commit the import repairs the compiler path and nothing else, and the control is raised the first time a UDT registers a type or arms an operator with C source. Also note that fix_jit_config() sets 'on' as well as repairing and probing.
eriknw
force-pushed
the
40-ss-import-jit-control
branch
from
September 24, 2026 20:07
4c75928 to
c5a45a8
Compare
_enable_jit_for_udt cached False when the first UDT op found jit_c_control at 'off' or 'pause', though an opt-out says nothing about whether the process can compile. A saved 'run' restored afterwards was then never raised, so every later UDT op ran without the JIT, silently. An opt-out is now honored without being cached, and the function returns whether a kernel armed now will be compiled. 'load' is now honored the same way instead of being raised. SuiteSparse sets it after a compile fails so that the failure is not repeated, and raising it at the next arming ran the failing compiler again at every later UDT op, raising JitError each time on SuiteSparse 9.4 and later. It also left 'off' as the only way to stop a host with cached kernels and no compiler from trying to compile. Also say in fix_jit_config's docstring that it sets 'on', and which SuiteSparse versions raise the JitError its probe absorbs. In the user guide, say that NoJITWarning covers only a UDT with no C form and which calls raise the control. Share one helper between the two subprocess tests, and word as hypotheticals the test docstrings for failures that never reached main.
Member
Author
|
Merging. I don't like some of the language such as "arming" (I wouldn't say this), but I'll do a bulk comments/docs cleanup later. The JITs are pretty complicated, but I think this helps make them "just work" for users at the time of need. |
eriknw
added a commit
that referenced
this pull request
Oct 1, 2026
…_new after #630 (#631) * Refresh the sparse pool and lift its stale Python 3.13 exclusion The pools refresh below this commit left sparse behind: the pool stopped at 0.15 while 0.16 through 0.19.2 exist, and py3.13/3.14 hardcoded "NA" under a comment saying sparse does not support them. That was true in the 0.15 era, when numba could not run on those Pythons; it is not true now. Constraints read off conda-forge metadata rather than assumed: every sparse release here is noarch with numba >=0.49 and numpy >=1.17 and no numpy ceiling, so only the Python floors matter. 0.16/0.17 declare python >=3.10 and 0.18/0.19 declare >=3.11. On py3.13/3.14 the pool starts at 0.16 because 0.14/0.15 predate those Pythons; on py3.11/3.12 the old entries stay. No new pairing rules are needed: the workflow already skips sparse whenever numba is skipped or NA, which covers sparse's only runtime dependency. The validate() Python check moves from "any sparse on py3.13+" to the 0.16 floor, and the stress test passes 160000 combos. Verified beyond metadata on this machine (py3.14, macOS arm64): sparse 0.16.0 and 0.19.2 pass the pydata-sparse io tests, and 0.18.0 passed the full suite. **Edit: rebased to include the changes on main, which largely supercedes this** * Test and allow every sparse release on Python 3.13 and 3.14 The CI pools started sparse at 0.16 on py3.13 and 0.18 on py3.14, and the io extra in pyproject.toml had the same floors. Each was the first sparse release to come out after that Python, on the assumption that older ones might not work there. They do. sparse is noarch, and every release from 0.14 on declares numba >=0.49, numpy >=1.17 and a Python floor of 3.11 or lower (>=3.8 for 0.14/0.15, >=3.10 for 0.16/0.17, >=3.11 for 0.18/0.19), with no ceiling on any of them. Each pin, as conda-forge resolves it (0.14.0, 0.15.5, 0.16.0, 0.17.0, 0.18.0 and 0.19.2), solves on both Pythons and passes the pydata-sparse tests with the oldest and newest numpy/numba pair each Python draws: numpy 2.1 with numba 0.62 and numpy 2.5 with numba 0.67 on py3.13, numpy 2.3 with numba 0.63 and numpy 2.5 with numba 0.67 on py3.14. So every Python now draws from the same pool, 0.14 through 0.19 plus latest, and the io extra requires sparse >=0.14 wherever numba installs. No pairing rules are needed, since the workflow already drops sparse whenever it skips numba. The stress test passes 160000 combinations, and both version audits pass. * Skip the failed-compile JIT test on SuiteSparse 8 test_failed_compile_is_not_retried_at_the_next_arming, from #630, arms binary.plus on a UDT with C source for SuiteSparse to compile, then makes that compile fail. An op is armed with C source only when the JIT string setters exist, and those arrived in SuiteSparse 9, so on 8.x jit_c_source is None and the test failed at its first assertion. It gets that far only where the JIT probe passes. In CI that happens only on some macOS runners that draw psg 8 (numpy 1.x); main's CI failed there after #630 merged. The test now skips when _has_jit_set is false, like the other tests that need an armed op. On 8.x the stand-in config tests still check that _enable_jit_for_udt leaves 'load' alone. Locally on psg 9.3.1.0, the only release in the CI pool that takes its pre-9.4 branch, it runs and passes. * Enable the JIT when registering from C source, and say why it failed gb.unary.ss.register_new and its binary, indexunary, select, and indexbinary siblings create the op from a C definition and no function pointer, so SuiteSparse has to compile the definition, or load it from its cache, before the op exists. Under its default jit_c_control of 'run' it does neither, and the registration fails with a bare NullPointer. Before #630, importing graphblas.ss raised the control to 'on', and the docstrings of these functions send users to gb.ss.config, so touching it first was enough; a process that never touched gb.ss already failed. #630 stopped the import from changing the control and moved the raise to _enable_jit_for_udt, which UDT op arming and gb.dtypes.ss.register_new call but these registrars did not. Since then nothing raises it for them, and each fails in a fresh process, on SuiteSparse 8.0.2 and 10.5 alike. The four op registrars and gb.dtypes.ss.register_new now create through one context manager, _compiling_c_source, which raises 'run' first and still honors an explicit 'off', 'pause', or 'load'. When creation fails anyway, SuiteSparse's error (NullPointer, InvalidValue, or from 9.4 JitError) has no message, so the same exception is raised again with the reason, read off jit_c_control before and after: the JIT is off or paused; it was left at 'load' and the kernel is not cached; this compile failed and SuiteSparse set 'load'; the compiler probe failed in this very call; or the control is still 'run' because no compiler was found or the library has no JIT. gb.dtypes.ss.register_new also accepted a name of GxB_MAX_NAME_LEN characters. That limit counts the NUL, so SuiteSparse cut the name by one, it no longer matched the typedef, and the compile failed. It is now rejected up front. The user guide's list of what raises the control includes the op registrars. New tests register one op of each kind in a fresh process, resetting the control to 'run' before each so none can rely on another, check each reason's message, and check the name limit. * Fix small errors in docstrings, messages, and docs Messages: Matrix, Vector, and Scalar __sizeof__ printed a literal {backend} (no f prefix); the Matrix diagonal size-mismatch message ran two sentences together, and the nrows/ncols error two words; the compression level error named only "lz4hc" where "zstd" also takes a level; the UDT repr size warning ended its first sentence twice. Docstrings: isequal documented check_dtype as check_dtypes; from_csc documented col_indices and said ncols is computed where it takes row_indices and computes nrows; from_dicts documented d for nested_dicts; Vector.diag documented a dtype it does not take; Vector.from_scalar documented nrows and ncols for size; serialize gave the zstd default level as 19 and a 1-9 range for both methods (zstd defaults to 1 and takes 1-19), and the descriptor docs gave the lz4hc default as 1 (it is 9); operator registration pointed at a graphblas.ops namespace, which is graphblas.op; several parameter lines numpydoc cannot parse ("name: type", "name :type"); two choice lists missing a closing quote; .mtz.gz for .mtx.gz in mmread; "`first` any `any`" in a comment. Docs: "the the" in the operators guide, the README converting the scipy matrix back instead of the GraphBLAS one, and two misspellings in a CSS comment.
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
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.
Stacked on #589.
Importing
graphblas.ssraisedjit_c_controlfrom SuiteSparse's default'run'to'on'. That submodule is reached by attribute access, soprint(gb.ss.about["library_version"]), the line a user writes for a bug report, silently changed the numerical machinery of every later call in the process.'on'is also what makes SuiteSparse load kernels from its on-disk cache, and that cache is keyed on the version triple alone. Two builds sharing a triple but differing in a JIT-visible header therefore share kernels: locally, kernels compiled by a 10.4.0 dated "July 31, 2026" were loaded by a 10.4.0 dated "Aug 7, 2026", whose GB_Matrix_content.h had moved a field. A four-element complement-masked assign returned GrB_OUT_OF_MEMORY, and a full suite run showed 25 unrelated-looking failures. Reported upstream with a standalone C reproducer.The import now repairs the conda-baked compiler path and nothing else. The bump to
'on'moves to_enable_jit_for_udt, called when a UDT registers a type or arms an op with C source: acts that plainly involve compiling C, so enabling the compiler is not a surprise. An explicit'off'or'pause'is honored; only SuiteSparse's own non-compiling defaults are raised.fix_jit_config()is unchanged in intent, since enabling compilation is what its caller asked for; it now also re-arms the cached probe answer, so a user-driven repair gets re-probed at the next UDT op instead of trusting a pre-repair verdict.Two details the hooks make necessary:
_probe_jitregisters a UDT to verify the compiler, and registering a UDT now asks to enable the JIT, which would probe again. The inner probe registered the probe type and the outer one then died redeclaring it through cffi, so a working environment gotFalsefrom the firstfix_jit_config()andTruefrom every call after._probing_jitmakes the re-entrant request answer yes without starting a second probe._maybe_warn_no_jitblamedjit_c_controlfor a UDT that is not expressible as a C struct. Codegen fails before anything arms a kernel, so the control is still'run'and the old message sent the reader to a setting that could not help. It now only blames the control once something has actually tried to enable the JIT.A second commit updates the UDT user guide, whose JIT section still said the import raises the control.