Repository navigation
fix(abi): keep device storage out of solver_settings_t, and check the layout - #2020
Draft
ramakrishnap-nv wants to merge 2 commits into
Draft
ramakrishnap-nv wants to merge 2 commits into
ramakrishnap-nv wants to merge 2 commits into
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
… layout
cuopt_mathopt constructs solver_settings_t and cuopt_client mutates it, so the
two libraries have to agree on its layout. They are built independently, and the
layout was a function of rmm, raft and CCCL headers:
solver_settings_t
-> pdlp_solver_settings_t
-> pdlp_warm_start_data_t by value -> 9 x rmm::device_uvector
The four parameter-registry vectors sit after those members, so any disagreement
about their size moved the registry. Two wheels built against different nightly
rmm wheels did exactly that, and set_parameter walked the registry from an
offset that was valid in one library and garbage in the other:
#1 cuOptSetFloatParameter libcuopt_mathopt.so
#0 solver_settings_t::set_parameter<double> libcuopt_client.so SIGSEGV
pdlp_warm_start_data_ is now held by shared_ptr, which is the same size whatever
the pointee looks like. sizeof(solver_settings_t) drops from 3248 to 2184, and
the nine device_uvector members leave the layout entirely. The pointer also
starts null, so the client no longer reaches across the library boundary to
construct device storage.
Smaller surface is not the same as a guarantee, so cuOptCreateSolverSettings now
compares its own sizeof against the value cuopt_client reports and returns
CUOPT_RUNTIME_ERROR with both numbers rather than writing through a wrong
offset. That also covers the case no build-side change can reach: a user
installing mismatched component wheels, which are not pinned to each other.
pdlp_warm_start_data_view_t and cuda::std::span remain in the layout. Both are
pointer-and-size types rather than containers, so the volatile part is gone, but
the layout is not yet free of device headers.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Cython bindings construct solver_settings_t directly via `new`, bypassing cuOptCreateSolverSettings entirely, so the sizeof guard added there never ran on that path. Move the check into the constructor itself, the one place both the C API and Cython glue actually go through. Also add an extern template declaration so no consumer other than solver_settings.cu (the explicit instantiation site) can generate its own local copy of the ctor/dtor -- the mechanism behind a newly reported destructor-side crash, distinct from the sizeof mismatch this already caught. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
ramakrishnap-nv
force-pushed
the
fix/settings-abi-stable
branch
from
October 5, 2026 19:02
5cd7e60 to
f1112f5
Compare
rapids-bot Bot
pushed a commit
that referenced
this pull request
Oct 6, 2026
…2051) Prevents a component wheel's build failure from leaving its last-published wheel stale next to freshly-published siblings -- the layout mismatch behind #2045's import segfault. Adds a `wheel-build-libcuopt-suite` checkpoint job requiring libcuopt, libcuopt-client, libcuopt-mathopt and libcuopt-routing to all build successfully, and makes every `wheel-publish-*` job for this family depend on it. Also fixes a pre-existing gap where `wheel-publish-libcuopt` and `wheel-publish-cuopt` didn't wait for their own declared dependencies to publish first, unlike libcuopt-mathopt/-routing which already waited on client. Complements #2020 (which fails loud on a mismatch that already happened) by reducing how often a mismatch can reach the index in the first place. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Authors: - Ramakrishna Prabhu (https://github.com/ramakrishnap-nv) Approvers: - Trevor McKay (https://github.com/tmckayus) URL: #2051
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
1 participant
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.
Fixes the crash in #2009.
cuopt_mathoptconstructssolver_settings_tandcuopt_clientmutates it, but the layout depended on rmm, raft and CCCL headers:The registry sits after those members, so two libraries built against different nightly rmm wheels disagreed about where it starts:
pdlp_warm_start_data_is held byshared_ptr, so its size no longer depends on rmm.sizeof(solver_settings_t)goes from 3248 to 2184. The pointer starts null, so the client stops constructing device storage across the library boundary.cuOptCreateSolverSettingscompares itssizeofagainst the valuecuopt_clientreports and returnsCUOPT_RUNTIME_ERRORwith both numbers instead of writing through a wrong offset. This also covers mismatched component wheels, which are not pinned to each other.Verified locally: full build clean,
NUMOPT_INTERNAL_TESTpassed (442s),LP_UNIT_TEST37/37, and the C API probe from #2009 now runs every step and returnsOptimal, objective-1.0.Not covered here:
pdlp_warm_start_data_view_tandcuda::std::spanare still in the layout. Both are pointer-and-size types rather than containers, so the volatile part is gone, but the layout is not yet free of device headers. The view is ~50 call sites and Cython-facing, so it is better done separately.🤖 Generated with Claude Code