GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |
GeezerVison is showing github.com — [pre-commit.ci] pre-commit autoupdate by pre-commit-ci[bot] · Pull Request #449 · python-graphblas/python-graphblas Conversationupdates: - https://github.com/myint/autoflake → https://github.com/PyCQA/autoflake - [github.com/PyCQA/autoflake: v2.0.2 → v2.1.1](PyCQA/autoflake@v2.0.2...v2.1.1) - [github.com/asottile/pyupgrade: v3.3.1 → v3.3.2](asottile/pyupgrade@v3.3.1...v3.3.2) - [github.com/charliermarsh/ruff-pre-commit: v0.0.261 → v0.0.263](astral-sh/ruff-pre-commit@v0.0.261...v0.0.263) - [github.com/charliermarsh/ruff-pre-commit: v0.0.261 → v0.0.263](astral-sh/ruff-pre-commit@v0.0.261...v0.0.263)
Member
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
eriknw
added a commit
that referenced
this pull request
…DT ops on mixed operand types (#652) * ss.fix_jit_config: find a conda compiler on Windows (Library/bin, .exe names) The conda repair looked for {baked-in name, cc, clang, gcc} in $CONDA_PREFIX/bin only. Conda on Windows installs compilers in %CONDA_PREFIX%\Library\bin, and their names end in .exe, so the repair found nothing there and left SuiteSparse's build-time compiler, which a user's machine does not have. It now looks in both places, with and without .exe, and takes a Windows path's basename with either slash. Tested with a fake prefix (no Windows machine at hand). * JIT C definitions for lifted ops on a scalar and a UDT, or on two UDT types A lifted binary op (plus, minus, times, truediv, floordiv, min, max) whose operands are a scalar and an array or record UDT, in either order, or two UDTs of one layout with other element types, ran only through its Numba cfunc: the C definition was set for same-type pairs alone, with one C type for x, y and z. So a semiring such as plus_times[FP64, FP64[R]] ran SuiteSparse's generic kernel with a function call per multiply. Such an op now gets a C definition with a type for each of z, x and y (udt_codegen._make_jit_c_mixed_definition), named op_N_x_y with N the length of x's C name, so that no two ops share a name (SuiteSparse finds a compiled kernel in memory by its op's name): a scalar operand is read as (*x) for every element or leaf, and each operand is converted in C to the type the op computes in, as the cfunc converts it (udt._udt_op_types). Ops with no such form keep the cfunc: an array operand that broadcasts, an element not computed in the result's type (integer truediv), a record with an array leaf, and eq/ne on a mixed pair. The whole mxv with plus_times[FP64, FP64[R]] now compiles as one JIT kernel (burble: dot2, jit). test_udt_scalar_broadcast_skips_jit becomes test_udt_scalar_broadcast_jit_routing: mixed arithmetic now JITs, mixed eq/ne still does not. * Test the mixed semiring's JIT kernel by burble's generic, absent and present The burble names a JIT kernel only when it compiles or loads it, so a kernel loaded earlier in the process prints no '(jit: ...)'. A UDT product has no kernel but the JIT one and the generic one, which burble names, so the test checks that 'generic' is absent with the JIT on and present with it off. * Vary the mixed JIT C name test's inputs, so its kernel runs on every SuiteSparse test_udt_mixed_jit_c_name_is_its_own built its vectors from constant values. SuiteSparse before 10.5 makes a vector built from equal values iso, and an ewise_mult of iso vectors gives an iso result computed without the JIT kernel the test is about, so on those versions it checked nothing. On them its to_dense also raised "TypeError: unsupported operand type(s) for +: 'int' and 'tuple'" (wave 2 CI, 17 jobs, e.g. 122633175811 and 122633175893), the iso array-UDT export bug fixed in #591. The values now differ by index. * Set the SuiteSparse JIT control at init, from a new config key Behavior change. Before: python-graphblas left jit_c_control where the SuiteSparse:GraphBLAS build started it ("run" in the conda-forge and PyPI builds, "on" in upstream's own), and UDT work raised "run" to "on" when it needed C compiled, whoever had set it. Turning the JIT on for everything meant setting gb.ss.config["jit_c_control"] after init. After: when the "suitesparse" backend initializes, it sets jit_c_control from the new config key graphblas.config["jit_c_control"]. The key is unset by default, which sets "run": nothing changes for the conda-forge and PyPI builds, and a build that starts at "on" now starts at "run" too, so every build has one documented default. GRAPHBLAS_JIT_C_CONTROL, gb.config.set before init, or a YAML file in ~/.config/graphblas/ set it; the values are "on", "load", "run", "pause" and "off" (YAML's unquoted on/off arrive as bools and are taken as such), and any other value raises ValueError before anything is initialized. UDT work still raises the default "run" to "on" when it needs C compiled, but now keeps a "run" the user set, as it already kept "off", "pause" and "load". A "run" set before init counts: GRAPHBLAS_JIT_C_CONTROL, gb.config.set or a YAML file, one key that donfig merges, so they could not be told apart and need not be; the environment variable is also the form that reaches every worker of a pool or cluster, which is what a "run" is for (GraphBLAS #449, below). A later gb.ss.config["jit_c_control"] = "run" counts when it changes the configured control (a "run" over a configured "on", say), and keeps a configured "run"; with nothing configured it is the default "run" again. The comparison is with the configured control, not the live one, which UDT work may have raised to "on": code that saves the default "run", sets something else and restores it would otherwise turn the default into a chosen "run" by accident. So, at runtime, "load" or "off" keep UDT work from switching to "on", as does GRAPHBLAS_JIT_C_CONTROL=run set before starting. gb.ss.config["jit_c_control"] = None is the default whatever is configured. Under a "run" that was set, UDT ops run their Numba functions (no compiled kernel, typically 2-3x slower for elementwise ops), and types and ops registered from C source (dtypes.ss.register_new and the op modules' ss.register_new) fail with a message that says the control was set to "run" and how to opt in: gb.ss.config["jit_c_control"] = "on", or None for the default. "on" makes SuiteSparse compile kernels it has no precompiled one for: Matrix.reduce_rowwise with lor on BOOL or plus on UINT64 measured 2.7-8x faster than the generic kernel. It stays opt-in because from SuiteSparse 9.4 on a failed compile returns GxB_JIT_ERROR, and processes that share one cache directory race (GraphBLAS #449; 10.1.1 dropped the file locks): on 10.5.2, 127 of 280 concurrent processes compiling the same new kernels into one cache got errors, invalidated or cleared outputs, wrong values or hangs, where 8.2.1, which locked the cache, had none. When "on" is asked for, it is set only where it works (SuiteSparse 7 has no JIT, and nothing is set there); elsewhere init sets "run", with no error or warning: a library built without the JIT (psg's wheels clamp writes to "run"), no compiler on disk after the conda repair, a cache directory it cannot write to (a probe cached there still loads, and the first new kernel would then fail to compile), or a compiler that fails a probe. Without the probe a broken compiler would fail the first user operation that needs a new kernel (a slice, say) with a bare JitError. The probe compiles a trivial type through GxB_Type_new; the kernel is cached, so after the first time per cache directory a probe is a load, about 2 ms against 0.3 s. The type is named after a hash of the compiler name, flags and libraries, so another toolchain sharing the cache gets a probe of its own. A failed probe sets "run", not the "load" SuiteSparse leaves, since "load" still reads kernels from the cache. UDT work makes the same writability check before it raises the default "run" to "on": without it (as before this change), a read-only cache that already held the probe let the raise through, and the first new UDT kernel failed with a bare JitError; now the UDT op runs its Numba function and the control stays "run". _probe_jit now goes through the C API for fix_jit_config and UDT work too, so it registers no dtypes.ss._jit_probe and needs no re-entrancy guard. Init can run in the middle of importing a core module, so it reads and writes these settings through the C API (_CConfig) instead of gb.ss.config, and jit_config imports ffi and lib inside the functions that use them: init imports it, so importing it first would otherwise start init halfway through its own import. Importing gb.ss still changes nothing (#630). * Docs: the SuiteSparse JIT control, how to opt in, and when "on" is unsafe A section in the initialization page of the user guide, about a screen long: what "on" buys (the 2.7-8x reduce numbers), that the default is "run" and that UDT work raises it but leaves a control the user set, how to opt in (GRAPHBLAS_JIT_C_CONTROL, gb.config, gb.ss.config) and the values, that unset is the default and a "run" set later in gb.ss.config is then that default again (at runtime "load" or "off", or GRAPHBLAS_JIT_C_CONTROL=run before starting, keep UDT work from switching to "on"), where "on" falls back to "run" with no error, and the costs of "on". The first is plain: processes running at the same time must not share a cache directory on SuiteSparse 10.1.1 and later, which no longer lock the cache (GraphBLAS #449: JitError, invalid or cleared outputs, wrong values returned with success, hangs). The simple remedy for multiprocessing, dask or pytest-xdist is GRAPHBLAS_JIT_C_CONTROL=run in the workers' environment, which holds for UDT work too; it costs UDT operators their compiled kernels (they run their Numba functions, typically 2-3x slower elementwise) and makes C-source registration fail. A cache per process keeps the kernels. The default will change once SuiteSparse publishes kernels atomically. Then the first-use compile latency, the cache keyed only on the version, and what to do on trouble: set GRAPHBLAS_JIT_C_CONTROL=run, delete the cache, report it. The UDT page says the control starts at the config value, "run" by default, that UDT work and C-source registration raise that default but keep a control the user set (a "run" when set before init), and what a "run" that was set costs there, and links to the section; graphblas.yaml, jit_config's module docstring and gb.ss.config's jit_c_control entry point there, and the entry says what None does. * Exclude the mixed op codegen's array leaf check from coverage A record with an array leaf has no C typedef, so the mixed op's C definition returns before it looks at the leaves. * test_global_config: None sets jit_c_control to the default "run" The test expected every key outside GlobalConfig._defaults to refuse None, but "Set the SuiteSparse JIT control at init, from a new config key" gave jit_c_control a setter in which None is the default "run" whatever graphblas.config has, and not a "run" the user chose (UDT work may still raise it to "on"). Check both, then put the control back as the test found it, and restore the user-set flag, which the round trip earlier in the test can change too. * Skip the unusable-compiler probe check where SuiteSparse compiles with cmake test_set_jit_c_control_at_init points jit_c_compiler_name at a directory and expects the probe to fail. SuiteSparse built with MSVC always compiles through cmake, and cmake's Visual Studio generator chooses the compiler itself, so on Windows the probe succeeds and the control stays 'on'. Every Windows job failed on it. * Say that the JIT stays at "run" on Windows, where SuiteSparse compiles with cmake SuiteSparse built with MSVC, as conda-forge builds it, compiles JIT kernels only through cmake. python-graphblas checks the configured C compiler path before it sets "on", and on conda-forge Windows that is the build machine's cl.exe, which no user has, so a configured "on" becomes "run" there. The user guide's JIT section and the jit_use_cmake entry of gb.ss.config say so. * Keep a UDT op's JIT C name within SuiteSparse's 127 characters SuiteSparse refuses a JIT C name of GxB_MAX_NAME_LEN (128) characters or more, but the definition was set anyway and taken under the empty name. A mixed op on two UDTs whose names add up to about 118 characters then compiled a kernel that calls no function: every output value was left uninitialized, with no error. A single-type op on a UDT named with 122 or more characters (on main too) failed with an empty JitError. Shorten a long mixed-op name to a digest (a length of 0 cannot come from the readable form, so the name stays the pair's own), and set the definition only when SuiteSparse took the name; otherwise the op keeps its cfunc. Found by review (r652f).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
ReviewersNo reviewsUh oh!
There was an error while loading. Please reload this page. AssigneesNo one assignedLabels
None yet
ProjectsMilestoneNo milestoneDevelopmentSuccessfully merging this pull request may close these issues. Uh oh!
There was an error while loading. Please reload this page. 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. | |
| You are visitor 01402503 — GeezerVison does not store the pages you look up. Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required. |