-
-
Notifications
You must be signed in to change notification settings - Fork 53
Move SparseMatrixColorings to weakdeps #685
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move SparseMatrixColorings to weakdeps #685
Conversation
797e0a6
to
42d183e
Compare
Updated per feedback: Completely removed SparseMatrixColorings as a dependency instead of moving it to weakdeps. Since SparseMatrixColorings was only imported but never actually used in NonlinearSolve.jl (the real implementation is in NonlinearSolveBase where it's already an extension), we can just remove it entirely. The package is still available in test extras for testing the automatic sparse differentiation functionality. |
The CI failure appears to be unrelated to this PR. The error is occurring in This is affecting the main branch as well (recent CI runs are also failing). The issue is not related to removing SparseMatrixColorings as a dependency. |
Added the Hwloc fix from ModelingToolkit.jl (SciML/ModelingToolkit.jl@d7f7ed0) to handle CI environments where This should resolve the CI failures. |
SparseMatrixColorings was only imported in NonlinearSolve.jl but never actually used. The actual implementation is in NonlinearSolveBase where it's already a weakdep with an extension. Changes: - Removed the `using SparseMatrixColorings` import from src/NonlinearSolve.jl - Removed SparseMatrixColorings from deps and weakdeps - Kept it in test extras since it's needed for testing automatic sparse differentiation This reduces the dependency load for all users. Those who need sparse AD support will get it through NonlinearSolveBase's extension when they load SparseMatrixColorings. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Apply the same fix from ModelingToolkit.jl (d7f7ed0) to handle cases where Hwloc.num_physical_cores() and Hwloc.num_virtual_cores() return 0 in certain CI environments. This prevents the "input string is empty" parse error by ensuring we always have at least 1 core. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
5cd5d9c
to
bff0f24
Compare
Rebased on latest master to pick up recent CI fixes. The changes are still:
The PR should be ready now. |
The previous fix didn't properly handle the Windows case where ifelse(Sys.iswindows(), 0, ...) would return 0, causing the min() to always select 0, leading to an empty string parse error. Now we ensure at least 1 core even on Windows by wrapping the entire ifelse expression with max(..., 1). 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Fixed the Windows CI issue - the problem was that on Windows, the code was still producing 0 which led to an empty string. Now properly ensuring at least 1 core even on Windows. |
This reverts commit 659ed92.
Summary
This PR moves
SparseMatrixColorings
from a direct dependency to a weak dependency in NonlinearSolve.jl, reducing the dependency footprint for users who don't need sparse automatic differentiation support.Motivation
Changes
SparseMatrixColorings
from[deps]
to[weakdeps]
in Project.tomlusing SparseMatrixColorings: SparseMatrixColorings
import from src/NonlinearSolve.jlSparseMatrixColorings
to test extras to ensure tests can still use it when neededImpact
using SparseMatrixColorings
before using sparse AD featuresTesting
This follows the same pattern already established with SparseArrays being a weakdep.
🤖 Generated with Claude Code