Skip to content

fix(routing): penalize missing required breaks in route cost#1197

Open
np96 wants to merge 2 commits into
NVIDIA:mainfrom
np96:fix/unreachable-break-infeasibility
Open

fix(routing): penalize missing required breaks in route cost#1197
np96 wants to merge 2 commits into
NVIDIA:mainfrom
np96:fix/unreachable-break-infeasibility

Conversation

@np96
Copy link
Copy Markdown
Contributor

@np96 np96 commented May 11, 2026

Description

Routes that miss a required break were reported as feasible (status=0) because the per-dimension break cost max(0, breaks - num_breaks) only penalizes excess. After eject_until_feasible removes break nodes, the dimension reports zero infeasibility and the solver returns the break-less route as success.

Fix: in route_t::view_t::compute_cost, after the per-dim loop, count breaks whose break_latest <= route_arrival_at_depot and add max(0, required - breaks_present) to the BREAK infeasibility cost. Routes missing a required break now correctly report status=1 (INFEASIBLE).

Issue

#1195 — problem A

Checklist

  • I am familiar with the Contributing Guidelines.
  • Testing
    • New or existing tests cover these changes
    • Added tests
  • Documentation
    • NA

@np96 np96 requested review from a team as code owners May 11, 2026 18:39
@np96 np96 requested review from Kh4ster, rg20 and tmckayus May 11, 2026 18:39
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 11, 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.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 11, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a0821d04-db4c-433f-b7f6-1c3da3e8fb71

📥 Commits

Reviewing files that changed from the base of the PR and between a146585 and 49831a5.

📒 Files selected for processing (2)
  • cpp/src/routing/route/route.cuh
  • python/cuopt/cuopt/tests/routing/test_vehicle_properties.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • python/cuopt/cuopt/tests/routing/test_vehicle_properties.py

📝 Walkthrough

Walkthrough

This PR adds logic in route cost computation to penalize missing required breaks when both BREAK and TIME dimensions are enabled, and adds a unit test that asserts a routing instance with an unreachable required break is infeasible.

Changes

Required Break Infeasibility Penalty

Layer / File(s) Summary
Break Cost Penalty Logic
cpp/src/routing/route/route.cuh
When both BREAK and TIME dimensions are active, compute_cost estimates depot arrival, counts required breaks whose break_latest ≤ arrival, reads route-present breaks (breaks_forward[*n_nodes]), and adds positive shortfall to infeasibility_cost[0][dim_t::BREAK].
Test Validation
python/cuopt/cuopt/tests/routing/test_vehicle_properties.py
Adds test_required_break_unreachable_is_infeasible, which builds a 3-node scenario with a required break in an infeasible time window, solves, and asserts the solver returns infeasible status.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding a penalty for missing required breaks in route cost computation.
Description check ✅ Passed The description is directly related to the changeset, explaining the bug, the fix, and referencing issue #1195 with test coverage details.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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.

Signed-off-by: Nikolai Poperechnyi <n.poperechnyi@gmail.com>
@np96 np96 force-pushed the fix/unreachable-break-infeasibility branch from a146585 to 2a4500a Compare May 12, 2026 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant