Skip to content

FJ numerical fix + fix clang build#835

Merged
rapids-bot[bot] merged 7 commits intoNVIDIA:release/26.02from
aliceb-nv:fj-numerical-fix
Feb 6, 2026
Merged

FJ numerical fix + fix clang build#835
rapids-bot[bot] merged 7 commits intoNVIDIA:release/26.02from
aliceb-nv:fj-numerical-fix

Conversation

@aliceb-nv
Copy link
Contributor

@aliceb-nv aliceb-nv commented Feb 6, 2026

This PR includes a fix for a numerical edge case bug in FJ, along with additional safeguards; changes have been made to ensure compilation works with clang.

Summary by CodeRabbit

  • Bug Fixes

    • Improved weight validation and clamping to ensure non-negative values in feasibility algorithms.
    • Enhanced boundary handling for edge cases in weight copying and updates.
    • Refined feasibility comparison to use inclusive saturation checks for consistency.
  • Chores

    • Suppressed compiler warnings and minor formatting adjustments.
    • Added an explicit template instantiation and updated copyright metadata.

@aliceb-nv aliceb-nv added this to the 26.02 milestone Feb 6, 2026
@aliceb-nv aliceb-nv requested a review from a team as a code owner February 6, 2026 15:03
@aliceb-nv aliceb-nv added bug Something isn't working non-breaking Introduces a non-breaking change labels Feb 6, 2026
@aliceb-nv aliceb-nv requested review from kaatish and rg20 February 6, 2026 15:03
@coderabbitai
Copy link

coderabbitai bot commented Feb 6, 2026

📝 Walkthrough

Walkthrough

Adds an explicit template instantiation, suppresses an unused warning with a maybe_unused attribute, enforces non-negativity and bounds checks in feasibility-jump weight updates (with finite checks), tightens feasibility comparisons to non-strict inequalities, and adds an unused-variable suppression in a local-search callback.

Changes

Cohort / File(s) Summary
Template Instantiation & Compiler Warning
cpp/src/dual_simplex/cuts.cpp, cpp/src/linear_programming/pdlp_constants.hpp
Adds template class knapsack_generation_t<int, double>; within DUAL_SIMPLEX_INSTANTIATE_DOUBLE and marks kernel_config_from_batch_size with [[maybe_unused]] (formatting adjusted).
Feasibility-jump: copy & left/right weight updates
cpp/src/mip/feasibility_jump/feasibility_jump.cu
Replaces direct assignments with local new_weight using boundary fallback (1.0 when idx >= old_size), adds isfinite and >= 0.0 checks, clamps to 0.0, and uses the validated new_weight for fj_weights, fj_left_weights, and fj_right_weights.
Feasibility-jump: kernel weight update
cpp/src/mip/feasibility_jump/feasibility_jump_kernels.cu
Clamps updated new_weight to be non-negative before writing to constraint and left/right weight arrays (non-negativity enforced inside kernel).
Feasibility logic & metadata
cpp/src/mip/feasibility_jump/feasibility_jump_impl_common.cuh
Clamps cstr_weight to non-negative, changes saturation checks from < to <= for both old and new lhs comparisons, and updates copyright year range.
Local-search callback minor fix
cpp/src/mip/local_search/local_search.cu
Adds (void)problem_ptr; in the CPU FJ scratch callback to suppress unused-variable warnings without changing behavior or capture semantics.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'FJ numerical fix + fix clang build' accurately captures the main objectives of the changeset, referring to both the FJ (Feasibility Jump) numerical bug fix and clang build compatibility improvements across multiple files.

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

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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link

@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.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
cpp/src/mip/local_search/local_search.cu (1)

85-92: ⚠️ Potential issue | 🔴 Critical

Fix lambda capture: problem_ptr is no longer in scope.
The callback now captures only population, but still uses problem_ptr in the log. This will not compile (or will bind to the wrong symbol if any). Capture this or a local pointer and reference context.problem_ptr.

🐛 Proposed fix
-    cpu_fj.fj_cpu->improvement_callback = [&population](f_t obj, const std::vector<f_t>& h_vec) {
+    cpu_fj.fj_cpu->improvement_callback = [this, &population](f_t obj, const std::vector<f_t>& h_vec) {
       population.add_external_solution(h_vec, obj, solution_origin_t::CPUFJ);
       if (obj < local_search_best_obj) {
         CUOPT_LOG_TRACE("******* New local search best obj %g, best overall %g",
-                        problem_ptr->get_user_obj_from_solver_obj(obj),
-                        problem_ptr->get_user_obj_from_solver_obj(
+                        context.problem_ptr->get_user_obj_from_solver_obj(obj),
+                        context.problem_ptr->get_user_obj_from_solver_obj(
                           population.is_feasible() ? population.best_feasible().get_objective()
                                                    : std::numeric_limits<f_t>::max()));
         local_search_best_obj = obj;
       }
     };

@aliceb-nv
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 45bd8ce into NVIDIA:release/26.02 Feb 6, 2026
100 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.

2 participants