Fixed lower bound in single thread mode

[nguidotti]

This PR fixes the incorrect computation of the lower bound when running with just a single thread. In the single_threaded_solve, we were creating a separated worker instance from the pool, such that its lower bounds was never consider when computing the global lower bound. This PR simply change the routine to use a worker from the pool.

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

[mlubin]

Any tests worth adding? Maybe the instance that triggered the issue?

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1b570877-4e4c-4be1-9dfe-916e06e492ba

📥 Commits

Reviewing files that changed from the base of the PR and between d4d1567 and 70bdbf4.

📒 Files selected for processing (1)

• cpp/tests/mip/miplib_test.cu

🚧 Files skipped from review as they are similar to previous changes (1)

• cpp/tests/mip/miplib_test.cu

---

📝 Walkthrough

Walkthrough

Lower-bound computation now takes the minimum of three sources (atomic ceiling, heap, worker pool). Single-threaded solve was refactored to use the worker pool (retrieve idle worker pointer) instead of a stack-allocated worker. A single-threaded MIP test and a new MPS dataset (dominating_set) were added.

Changes

Cohort / File(s)	Summary
Branch & Bound core cpp/src/branch_and_bound/branch_and_bound.cpp	get_lower_bound() now computes and returns the minimum of lower_bound_ceiling_, node_queue_.get_lower_bound(), and worker_pool_.get_lower_bound() using intermediate variables; single_threaded_solve() was refactored to initialize worker_pool_ for one worker, obtain an idle worker pointer and use pointer-based calls (worker->...) and plunge_with(worker); removed unused launched_any_task variable.
Tests cpp/tests/mip/miplib_test.cu	Added mip_solve.low_thread_count_test that runs a single-threaded MIP solve (num_cpu_threads = 1, time_limit = 30) on mip/dominating_set.mps, asserts optimal termination and objective == 3.0, and validates variable bounds via existing helpers.
Datasets datasets/mip/dominating_set.mps	Added new MPS file defining a 9-variable binary/integer dominating set model with constraints c0–c8, RHS = 1, objective obj, and MARKER sections bracketing integer variables.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.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
Title check	✅ Passed	The title 'Fixed lower bound in single thread mode' directly matches the main objective of the changeset: fixing the global lower bound computation in single_threaded_solve by using a worker from the pool.
Description check	✅ Passed	The description is directly related to the changeset, explaining the bug (separate worker instance outside pool), the fix (use worker from pool), and its impact on lower bound computation.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[nguidotti]

Good idea. @chris-maes I can use the instance that you send?

[chris-maes]

Thanks for the quick fix @nguidotti

[chris-maes]

Good idea. @chris-maes I can use the instance that you send?

Yes. I think it's fine to use. It's a sample problem from page 12 of: https://coral.ise.lehigh.edu/~pubs/files/ostrowski_phd_thesis.pdf

[nguidotti]

/ok to test 70bdbf4

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

cpp/tests/mip/miplib_test.cu (2)

73-73: Nit: stray empty comment.

Line 73 is a lone // with no content — drop it.

Proposed cleanup

-//
 TEST(mip_solve, low_thread_count_test)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cpp/tests/mip/miplib_test.cu` at line 73, Remove the stray empty comment
(`//`) found in the test file (the lone comment line in miplib_test.cu) — delete
that line so the file has no empty comment, ensure no leftover trailing
whitespace on that line, and run the repository formatter/linter to confirm no
style violations remain.

---

88-89: Verify EXPECT_DOUBLE_EQ is appropriate here.

dominating_set.mps with optimum 3 should yield an exact integer objective, so bitwise equality is typically safe. However, if the solver reports the objective through any floating-point accumulation (e.g., summing c_j * x_j over the LP relaxation/reconstruction), tiny rounding could cause flakes. Consider EXPECT_NEAR(..., 3.0, 1e-9) to be robust without weakening the test, consistent with the epsilon-comparison guideline for floating-point equality checks.

As per coding guidelines: "use epsilon comparisons for floating-point equality checks".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cpp/tests/mip/miplib_test.cu` around lines 88 - 89, Replace the exact
floating-point equality check EXPECT_DOUBLE_EQ(solution.get_objective_value(),
3.0) with an epsilon comparison to avoid flakiness from tiny rounding; use
EXPECT_NEAR(solution.get_objective_value(), 3.0, 1e-9) (leave the existing
termination check EXPECT_EQ(solution.get_termination_status(),
mip_termination_status_t::Optimal) unchanged).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cpp/tests/mip/miplib_test.cu`:
- Around line 76-78: The test currently hardcodes settings.time_limit = 30 which
can be too short under ASSERT_MODE; update the code that constructs
mip_solver_settings_t<int,double> settings (variable settings) to conditionally
set settings.time_limit to 60 when ASSERT_MODE is enabled (same pattern used
earlier in test_miplib_file), so debug/assert builds use a 60s time_limit and
release builds keep 30s.

---

Nitpick comments:
In `@cpp/tests/mip/miplib_test.cu`:
- Line 73: Remove the stray empty comment (`//`) found in the test file (the
lone comment line in miplib_test.cu) — delete that line so the file has no empty
comment, ensure no leftover trailing whitespace on that line, and run the
repository formatter/linter to confirm no style violations remain.
- Around line 88-89: Replace the exact floating-point equality check
EXPECT_DOUBLE_EQ(solution.get_objective_value(), 3.0) with an epsilon comparison
to avoid flakiness from tiny rounding; use
EXPECT_NEAR(solution.get_objective_value(), 3.0, 1e-9) (leave the existing
termination check EXPECT_EQ(solution.get_termination_status(),
mip_termination_status_t::Optimal) unchanged).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0e00f81d-0713-44f1-b4f9-e5d023176062

📥 Commits

Reviewing files that changed from the base of the PR and between 6425c25 and d4d1567.

📒 Files selected for processing (2)

• cpp/tests/mip/miplib_test.cu
• datasets/mip/dominating_set.mps

✅ Files skipped from review due to trivial changes (1)

• datasets/mip/dominating_set.mps

[nguidotti]

/merge