Skip to content

Stop gb.ss import from turning the SuiteSparse JIT on - #630

Merged
eriknw merged 3 commits into
mainfrom
40-ss-import-jit-control
Sep 26, 2026
Merged

eriknw merged 3 commits into
mainfrom
40-ss-import-jit-control

Conversation

@eriknw

@eriknw eriknw commented Aug 26, 2026 •

Copy link
Copy Markdown
Member

Stacked on #589.

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 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_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. 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.

@eriknw
eriknw force-pushed the 40-ss-import-jit-control branch 4 times, most recently from e3f45f4 to 6ed0f67 Compare August 27, 2026 16:46
@eriknw
eriknw force-pushed the 40-ss-import-jit-control branch from 6ed0f67 to f9d2799 Compare September 18, 2026 03:25
@eriknw
eriknw force-pushed the 40-ss-import-jit-control branch from f9d2799 to 854fce4 Compare September 18, 2026 03:46
@eriknw
eriknw changed the base branch from 39-repr-chop-complex-overflow to 09-array-udt-udf-views September 23, 2026 18:18
@eriknw
eriknw force-pushed the 40-ss-import-jit-control branch from 854fce4 to 2082505 Compare September 23, 2026 18:19
@eriknw
eriknw added this pull request to stack #634 September 23, 2026 18:19
Base automatically changed from 09-array-udt-udf-views to main September 24, 2026 20:05
@eriknw
eriknw force-pushed the 40-ss-import-jit-control branch from 2082505 to 4c75928 Compare September 24, 2026 20:06
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
eriknw force-pushed the 40-ss-import-jit-control branch from 4c75928 to c5a45a8 Compare September 24, 2026 20:07
_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.
@eriknw

eriknw commented Sep 26, 2026

Copy link
Copy Markdown
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
eriknw merged commit f33f8c6 into main Sep 26, 2026
16 checks passed
@eriknw
eriknw deleted the 40-ss-import-jit-control branch September 26, 2026 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant