Skip to content

Fix more reference lifetime issues with set_value_async - #2202

Merged
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
mlubin:fix/async-scalar-lifetimes
Oct 23, 2026
Merged

rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
mlubin:fix/async-scalar-lifetimes

Conversation

@mlubin

@mlubin mlubin commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Similar issues as in #2200.

@mlubin
mlubin requested a review from a team as a code owner October 23, 2026 01:22
@copy-pr-bot

copy-pr-bot Bot commented Oct 23, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@mlubin
mlubin force-pushed the fix/async-scalar-lifetimes branch from f03d181 to d0a6dc9 Compare October 23, 2026 01:23
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: NVIDIA/cuopt
  • Branch: main
  • File: .coderabbit.yaml
  • Commit: 6cce8f7
  • Layers: Repository YAML
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 6f1115e6-9f2d-4253-9ce5-b493e6f6c019
📥 Commits

Reviewing files that changed from the base of the PR and between d0a6dc9 and 782f16c.

📒 Files selected for processing: 1
  • cpp/src/barrier/cusparse_info.hpp

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

📝 Walkthrough

The cuSPARSE info constructor now initializes alpha and beta to one on the handle stream. The PDLP restart strategy now uses cub::DeviceTransform::Fill to set its device-side iteration count.

Changes: cuSPARSE Scalar Initialization

Layer / File(s) Summary
Initialize cuSPARSE scalars
cpp/src/barrier/cusparse_info.hpp
The constructor initializes alpha and beta to one on the handle stream and removes the subsequent asynchronous writes.

PDLP Iteration Counter Update

Layer / File(s) Summary
Fill the iteration counter
cpp/src/pdlp/restart_strategy/weighted_average_solution.cu
set_iterations_since_last_restart uses cub::DeviceTransform::Fill for the device-side count. The host-side assignment is unchanged.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: chris-maes, bubullzz

Merge Risk: ⚪ Minimal · up to 782f1

No current-PR merge-blocking risk was verified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies fixes for reference lifetime issues related to set_value_async, which matches the changes.
Description check Passed The description links the changes to similar issues in #2200, so it is related to the changeset.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@mlubin mlubin added bug Something isn't working non-breaking Introduces a non-breaking change labels Oct 23, 2026
@mlubin

mlubin commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test d0a6dc9

Initialize cuSPARSE alpha and beta from static one constants, and use CUB Fill to pass the PDLP restart iteration count by value. This avoids asynchronous copies borrowing constructor-local or setter-local values.

Signed-off-by: Miles Lubin <mlubin@nvidia.com>
@mlubin
mlubin force-pushed the fix/async-scalar-lifetimes branch from d0a6dc9 to 782f16c Compare October 23, 2026 01:27
@mlubin

mlubin commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 782f16c

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

CI Test Summary

1 failed · 20 passed · 0 skipped

  • wheel-tests-cuopt-server / 13.3.0, 3.14, amd64, ubuntu24.04, h100, latest-driver, latest-deps — failed; no test names found in log (job log)

@mlubin

mlubin commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

/merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants