Fix explored nodes counter

[nguidotti]

This PR changes how the explored node counter is updated, such that it is only incremented after solving a node in the B&B.

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

Summary by CodeRabbit

• Refactor
  • Improved progress logging and node counting coordination in the branch-and-bound solver algorithm.

[coderabbitai]

Walkthrough

The branch-and-bound algorithm's node counting and progress logging flow has been refactored. Node counter updates (nodes_explored, nodes_unexplored, nodes_since_last_log) are repositioned to occur around cutoff handling rather than per-step. A new should_report_ coordination flag gates logging behavior, with logging state reset after each report emission.

Changes

Cohort / File(s)	Summary
Header member addition cpp/src/dual_simplex/branch_and_bound.hpp	Added atomic boolean member should_report_ after status_ member for thread-coordinated reporting control
Node counting and logging refactor cpp/src/dual_simplex/branch_and_bound.cpp	Repositioned node counter updates (nodes_explored, nodes_unexplored, nodes_since_last_log) to occur around cutoff handling in multiple code paths (exploration_ramp_up, explore_subtree, solve initialization); integrated should_report_ coordination flag to gate progress logging; logging block now computes and displays user-objective gap metrics, then resets state and re-enables reporting

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• exploration_ramp_up function: Verify counter update placement around cutoff logic and consistency across branches
• should_report_ coordination: Ensure thread-safe access and proper state transitions across cutoff and logging paths
• Counter update consistency: Confirm nodes_explored, nodes_unexplored, and nodes_since_last_log increments/decrements are applied uniformly across all execution paths (cutoff path, post-exploration, logging emission)
• Logging state reset logic: Validate that logging state is properly reset after report emission and should_report_ is correctly re-enabled

Poem

🐰 A counter hop, a log reset,
Where threads report their progress yet,
With cutoff gates and flags aligned,
Coordination's bloom we find!
Through branches swift, the metrics flow—
Watch 'em bounce and watch 'em grow! 🌱

Pre-merge checks and finishing touches

❌ 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%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ 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 'Fix explored nodes counter' directly describes the main change: modifications to how the explored node counter is updated in the branch-and-bound solver, aligning with the PR's stated objectives.

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[nguidotti]

/ok to test bd697c9

[coderabbitai]

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)

cpp/src/dual_simplex/branch_and_bound.cpp (1)

723-737: Use the current lower bound when logging

Here we print progress using user_lower = compute_user_objective(original_lp_, root_objective_). That root objective is the initial LP bound, so after the tree tightens the lower bound the reported gap never reflects it and the metric stays artificially wide. Please base user_lower (and the gap) on the current lower bound you already computed for this node.

diff --git a/cpp/src/dual_simplex/branch_and_bound.cpp b/cpp/src/dual_simplex/branch_and_bound.cpp
@@
-      f_t obj              = compute_user_objective(original_lp_, upper_bound);
-      f_t user_lower       = compute_user_objective(original_lp_, root_objective_);
+      f_t obj              = compute_user_objective(original_lp_, upper_bound);
+      f_t user_lower       = compute_user_objective(original_lp_, lower_bound);

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bc49f7a and 9734677.

📒 Files selected for processing (2)

• cpp/src/dual_simplex/branch_and_bound.cpp (8 hunks)
• cpp/src/dual_simplex/branch_and_bound.hpp (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

cpp/src/dual_simplex/branch_and_bound.cpp (2)

cpp/src/dual_simplex/branch_and_bound.hpp (6)

• node (89-94)
• node (89-89)
• node (96-103)
• node (96-98)
• search_tree (233-237)
• search_tree (258-264)

cpp/src/dual_simplex/solve.cpp (6)

• compute_user_objective (98-103)
• compute_user_objective (98-98)
• compute_user_objective (106-110)
• compute_user_objective (106-106)
• compute_user_objective (650-651)
• compute_user_objective (653-653)

[chris-maes]

This is a bit strange. I think of nodes explored as the number of nodes we have solved, or perhaps proved infeasible through node presolve. If we are fathoming the node here I would not count it as explored.

[chris-maes]

Same here. The purpose of nodes since last log is for us to print every so often in a deterministic way. Here we did no work, so I don't think we want to increase this counter.

[chris-maes]

Do we need to have member variable to decide if we should log? I think this is increasing the complexity of the code for little return.

[chris-maes]

Nit: is there a reason why all of these are ++x instead x++?

[chris-maes]

Why are we incrementing the nodes explored here at all?

[chris-maes]

Let's not count a node as explored if we trivially fathom it.

Let's talk offline about why you increment the nodes explored at the end of explore_subtree.

[github-actions]

🔔 Hi @anandhkb, this pull request has had no activity for 7 days. Please update or let us know if it can be closed. Thank you!

If this is an "epic" issue, then please add the "epic" label to this issue.
If it is a PR and not ready for review, then please convert this to draft.
If you just want to switch off this notification, then use the "skip inactivity reminder" label.