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
added this pull request to stack #634
September 23, 2026 18:19
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. |
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.