Skip to content

Update to PSLP version with bug fixes#920

Merged
rapids-bot[bot] merged 1 commit intoNVIDIA:mainfrom
rg20:update_pslp
Mar 3, 2026
Merged

Update to PSLP version with bug fixes#920
rapids-bot[bot] merged 1 commit intoNVIDIA:mainfrom
rg20:update_pslp

Conversation

@rg20
Copy link
Copy Markdown
Contributor

@rg20 rg20 commented Mar 2, 2026

Description

Updating to latest version of PSLP. This fixes a bug where PSLP is making feasible LP problems to infeasible ones and has performance improvements as well.

Issue

Checklist

  • I am familiar with the Contributing Guidelines.
  • Testing
    • New or existing tests cover these changes
    • Added tests
    • Created an issue to follow-up
    • NA
  • Documentation
    • The documentation is up to date with these changes
    • Added new documentation
    • NA

@rg20 rg20 requested review from a team as code owners March 2, 2026 19:28
@rg20 rg20 requested a review from tmckayus March 2, 2026 19:28
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot bot commented Mar 2, 2026

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.

@rg20 rg20 added bug Something isn't working non-breaking Introduces a non-breaking change labels Mar 2, 2026
@rg20 rg20 added this to the 26.04 milestone Mar 2, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 2, 2026

📝 Walkthrough

Walkthrough

Updated the PSLP dependency version from v0.0.4 to v0.0.8 in a FetchContent_Declare block. No control flow changes, error handling modifications, or other components affected.

Changes

Cohort / File(s) Summary
Dependency Version Update
cpp/CMakeLists.txt
Updated PSLP GIT_TAG from v0.0.4 to v0.0.8 in FetchContent_Declare block.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Update to PSLP version with bug fixes' clearly summarizes the main change in the changeset - updating a dependency version.
Description check ✅ Passed The description is related to the changeset, explaining the purpose of the PSLP version update and its benefits including bug fixes and performance improvements.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
cpp/CMakeLists.txt (1)

238-238: Pin PSLP to an immutable commit instead of a mutable tag.

At line 238, using GIT_TAG "v0.0.8" creates a reproducibility risk: Git allows tags to be force-moved or recreated upstream, making builds non-deterministic. CMake recommends pinning to the resolved commit SHA for deterministic and supply-chain-safe builds.

Proposed CMake change
 FetchContent_Declare(
   pslp
   GIT_REPOSITORY "https://github.com/dance858/PSLP.git"
-  GIT_TAG "v0.0.8"
+  # v0.0.8 resolves to commit 91404985d5fcf560637ec522e1356c3ed1c43357
+  GIT_TAG "91404985d5fcf560637ec522e1356c3ed1c43357"
   GIT_PROGRESS TRUE
   EXCLUDE_FROM_ALL
   SYSTEM
 )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cpp/CMakeLists.txt` at line 238, The CMake fetch currently pins PSLP with a
mutable tag via GIT_TAG "v0.0.8"; replace that mutable tag with the repository's
immutable commit SHA (the resolved commit for v0.0.8) so builds are
deterministic. Locate the FetchContent_Declare (or
add_subdirectory/ExternalProject) stanza that uses GIT_TAG "v0.0.8" and change
the GIT_TAG value to the exact commit SHA (the full 40-char hash) for that
release; ensure any reviewers/CI are updated to use the same SHA rather than the
tag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@cpp/CMakeLists.txt`:
- Line 238: The CMake fetch currently pins PSLP with a mutable tag via GIT_TAG
"v0.0.8"; replace that mutable tag with the repository's immutable commit SHA
(the resolved commit for v0.0.8) so builds are deterministic. Locate the
FetchContent_Declare (or add_subdirectory/ExternalProject) stanza that uses
GIT_TAG "v0.0.8" and change the GIT_TAG value to the exact commit SHA (the full
40-char hash) for that release; ensure any reviewers/CI are updated to use the
same SHA rather than the tag.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f399282 and b066ab2.

📒 Files selected for processing (1)
  • cpp/CMakeLists.txt

@rg20
Copy link
Copy Markdown
Contributor Author

rg20 commented Mar 2, 2026

/ok to test b066ab2

Copy link
Copy Markdown
Contributor

@anandhkb anandhkb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rg20
Copy link
Copy Markdown
Contributor Author

rg20 commented Mar 3, 2026

/merge

@rapids-bot rapids-bot bot merged commit a92fd23 into NVIDIA:main Mar 3, 2026
160 of 173 checks passed
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.

4 participants