Clean-up Logs

[akifcorduk]

This PR cleans up the logs for scaling.

[coderabbitai]

📝 Walkthrough

Walkthrough

Two files modified with logging adjustments. Negative gap error logging downgraded to debug level in branch and bound computation. Multiple objective and row-scaling messages reduced from info to debug level, and a scaling information print call was removed.

Changes

Cohort / File(s)	Summary
Logging Level Adjustments cpp/src/branch_and_bound/branch_and_bound.cpp	Downgraded negative gap condition logging from CUOPT_LOG_ERROR to CUOPT_LOG_DEBUG in compute_user_abs_gap.
Scaling Strategy Logging & Output cpp/src/mip_heuristics/mip_scaling_strategy.cu	Downgraded objective and row-scaling messages from CUOPT_LOG_INFO to CUOPT_LOG_DEBUG, removed print_scaling_information() call after row scaling completion, and adjusted indentation on debug log statements.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

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.
Title check	❓ Inconclusive	The title 'Clean-up Logs' is vague and generic, using non-descriptive terms that don't convey meaningful information about the specific changeset.	Provide a more specific title that indicates the actual changes, such as 'Downgrade scaling logs from INFO to DEBUG level' or 'Reduce logging verbosity in branch and bound and scaling modules'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description 'This PR cleans up the logs for scaling' is related to the changeset but is vague and doesn't provide specifics about what was changed.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

🧹 Nitpick comments (2)

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

183-183: Keep negative-gap anomaly visible above DEBUG.

At Line 183, moving "Gap is negative" to CUOPT_LOG_DEBUG makes bound-order anomalies easy to miss in standard logging configurations. This is still useful health telemetry for diagnosing incorrect bound progression; consider WARN (or rate-limited ERROR) instead.

Proposed logging-level adjustment

-  if (gap < -1e-4) { CUOPT_LOG_DEBUG("Gap is negative %e", gap); }
+  if (gap < -1e-4) { CUOPT_LOG_WARN("Gap is negative %e", gap); }

🤖 Prompt for AI Agents

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

In `@cpp/src/branch_and_bound/branch_and_bound.cpp` at line 183, The check in
branch_and_bound.cpp that logs a negative gap currently uses CUOPT_LOG_DEBUG for
the message "Gap is negative", which hides important bound-order anomalies;
change the log level in the if (gap < -1e-4) branch to a visible level such as
CUOPT_LOG_WARN (or a rate-limited CUOPT_LOG_ERROR) and include the gap value in
the message so the negative-gap anomaly is easily noticed during normal runs;
update the logging call in that if block (referencing the gap variable and the
existing conditional) accordingly.

cpp/src/mip_heuristics/mip_scaling_strategy.cu (1)

313-313: Preserve key scaling breadcrumbs at INFO (not only DEBUG).

These changes push most “why skipped / what happened” scaling messages to DEBUG. With common INFO-level runs, root-cause context disappears even though scaling materially affects conditioning and solve behavior. Consider keeping start / skip reason / summary at INFO, while leaving per-iteration metrics at DEBUG.

Suggested INFO/DEBUG split

-  CUOPT_LOG_DEBUG("MIP_OBJ_SCALING skipped: disabled by user setting");
+  CUOPT_LOG_INFO("MIP_OBJ_SCALING skipped: disabled by user setting");

-  CUOPT_LOG_DEBUG("MIP row scaling start: rows=%d cols=%d max_iterations=%d soft_big_m_rows=%d",
+  CUOPT_LOG_INFO("MIP row scaling start: rows=%d cols=%d max_iterations=%d soft_big_m_rows=%d",
                   n_rows,
                   n_cols,
                   row_scaling_max_iterations,
                   big_m_rows);

-      CUOPT_LOG_DEBUG("MIP row scaling skipped: initial_log2_spread=%g threshold=%g",
+      CUOPT_LOG_INFO("MIP row scaling skipped: initial_log2_spread=%g threshold=%g",
                       row_log2_spread,
                       row_scaling_min_initial_log2_spread);

-  CUOPT_LOG_DEBUG("MIP_SCALING_SUMMARY rows=%d bigm_rows=%d final_spread=%g",
+  CUOPT_LOG_INFO("MIP_SCALING_SUMMARY rows=%d bigm_rows=%d final_spread=%g",
                   n_rows,
                   big_m_rows,
                   previous_row_log2_spread);

Also applies to: 318-320, 329-331, 347-347, 508-508, 568-572, 618-620, 774-774, 792-794, 835-838

🤖 Prompt for AI Agents

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

In `@cpp/src/mip_heuristics/mip_scaling_strategy.cu` at line 313, Change the
high-level "start/skip/summary" scaling messages from CUOPT_LOG_DEBUG to
CUOPT_LOG_INFO so callers keep root-cause breadcrumbs (e.g., the
"MIP_OBJ_SCALING skipped: no finite nonzero objective coefficients" log and
other skip/start summary messages in the same file), while leaving per-iteration
and detailed metric prints at CUOPT_LOG_DEBUG; update the logging calls (replace
CUOPT_LOG_DEBUG with CUOPT_LOG_INFO) only for the messages that convey why
scaling was skipped or a summary outcome, and preserve DEBUG for
iterative/detail logs referenced in the review.

🤖 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/src/branch_and_bound/branch_and_bound.cpp`:
- Line 183: The check in branch_and_bound.cpp that logs a negative gap currently
uses CUOPT_LOG_DEBUG for the message "Gap is negative", which hides important
bound-order anomalies; change the log level in the if (gap < -1e-4) branch to a
visible level such as CUOPT_LOG_WARN (or a rate-limited CUOPT_LOG_ERROR) and
include the gap value in the message so the negative-gap anomaly is easily
noticed during normal runs; update the logging call in that if block
(referencing the gap variable and the existing conditional) accordingly.

In `@cpp/src/mip_heuristics/mip_scaling_strategy.cu`:
- Line 313: Change the high-level "start/skip/summary" scaling messages from
CUOPT_LOG_DEBUG to CUOPT_LOG_INFO so callers keep root-cause breadcrumbs (e.g.,
the "MIP_OBJ_SCALING skipped: no finite nonzero objective coefficients" log and
other skip/start summary messages in the same file), while leaving per-iteration
and detailed metric prints at CUOPT_LOG_DEBUG; update the logging calls (replace
CUOPT_LOG_DEBUG with CUOPT_LOG_INFO) only for the messages that convey why
scaling was skipped or a summary outcome, and preserve DEBUG for
iterative/detail logs referenced in the review.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 36be4476-5182-4d8d-b6d0-6d34b696d750

📥 Commits

Reviewing files that changed from the base of the PR and between 140c684 and b2895d9.

📒 Files selected for processing (2)

• cpp/src/branch_and_bound/branch_and_bound.cpp
• cpp/src/mip_heuristics/mip_scaling_strategy.cu

[chris-maes]

/merge