Skip to content

Routing Phase 3: export the recorded problem to a host representation#1583

Merged
rapids-bot[bot] merged 3 commits into
mainfrom
routing-cpu-api-phase3
Jul 20, 2026
Merged

Routing Phase 3: export the recorded problem to a host representation#1583
rapids-bot[bot] merged 3 commits into
mainfrom
routing-cpu-api-phase3

Conversation

@ramakrishnap-nv

@ramakrishnap-nv ramakrishnap-nv commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds to_host_problem(dm): exports a recorded routing problem (the store-then-build IR from #1556) to host numpy arrays keyed by the RoutingProblem fields, copying any device (cuDF) inputs to host. This is the client-side foundation for serializing a routing problem over gRPC.

Validated by a round-trip against the VRP RoutingProblem proto; full routing suite passes.

🤖 Generated with Claude Code

Add to_host_problem(dm): walks the store-then-build IR (DataModel._calls) and
produces host (numpy) arrays keyed by the gRPC RoutingProblem field names,
exporting any device (cuDF/cupy) inputs to host at this point (the mixed-IR
"export on serialize"). Proto-agnostic -- a dict of host arrays -- so it does
not depend on generated gRPC stubs and is the client-side foundation for
serializing a routing problem over gRPC. Covers the full setter surface,
including uniform and per-vehicle break dimensions.

Verified end-to-end: host+device (numpy/cuDF) inputs export to a fully host
problem with correct row-major matrices, grouped per-vehicle breaks, and no
device references left; full routing suite passes (56).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
@ramakrishnap-nv ramakrishnap-nv added this to the 26.08 milestone Jul 16, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jul 16, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

Replace the per-setter if/elif chain in to_host_problem with a dispatch table:
setters that rename fields, take several args, or build nested structures get
an explicit handler in _HANDLERS; the common set_<field>(array) -> field
<field> setters are derived automatically and need no entry. An unmapped add_*
setter raises, and test_every_setter_is_exportable checks every recorded setter
is exportable, so a new setter fails loudly instead of being silently dropped.
Behavior is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
@ramakrishnap-nv

Copy link
Copy Markdown
Collaborator Author

@CodeRabbit review

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

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: 144b62d0-2aac-4d40-a81b-ef3e51888cee

📥 Commits

Reviewing files that changed from the base of the PR and between 559b0ec and 1977898.

📒 Files selected for processing (2)
  • python/cuopt/cuopt/routing/_serialize.py
  • python/cuopt/cuopt/tests/routing/test_serialize.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • python/cuopt/cuopt/tests/routing/test_serialize.py
  • python/cuopt/cuopt/routing/_serialize.py

📝 Walkthrough

Walkthrough

Changes

Routing serialization

Layer / File(s) Summary
Host conversion and setter handlers
python/cuopt/cuopt/routing/_serialize.py
Converts array-like inputs to host NumPy arrays and maps specialized routing setters into nested and accumulated export structures.
Recorded problem export
python/cuopt/cuopt/routing/_serialize.py
Builds the host problem from initialization arguments and recorded calls, applies default setter mappings, initializes list fields, and rejects unmapped or unsupported setters.
Serialization coverage and validation
python/cuopt/cuopt/tests/routing/test_serialize.py
Validates setter export coverage, host-only output, routing values and metadata, and uniform and vehicle-specific break serialization.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: exporting the recorded routing problem to a host representation.
Description check ✅ Passed The description directly matches the changeset by describing to_host_problem and host-side export of routing data.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch routing-cpu-api-phase3

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@python/cuopt/cuopt/routing/_serialize.py`:
- Around line 180-184: Update the public function to_host_problem with explicit
parameter and return type annotations, using the project’s existing types for
the routing problem input and host-array mapping. Expand its docstring to
document the dm parameter, returned RoutingProblem-keyed host arrays, and the
KeyError raised when required problem fields are missing.
- Around line 194-205: Update the unregistered set_* fallback in the _serialize
loop to require exactly one argument before exporting args[0]; raise the
existing unmapped-setter error (or equivalent) for multi-argument calls so they
require an explicit _HANDLERS entry. Add a regression test covering a
multi-argument set_* call and verify it is rejected.

In `@python/cuopt/cuopt/tests/routing/test_serialize.py`:
- Around line 61-77: Strengthen the routing serialization assertions in
python/cuopt/cuopt/tests/routing/test_serialize.py lines 61-77 by validating
time-window arrays, both capacity demand/capacity arrays, and matrix dtype plus
metadata alongside values, shapes, and existing fields. Also update lines 94-103
to assert uniform-break earliest/latest and every field of both vehicle breaks,
including both locations arrays; retain the existing device-reference check.
🪄 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: Enterprise

Run ID: 924e8e2a-f40f-4a0a-9a2c-9a10478710cf

📥 Commits

Reviewing files that changed from the base of the PR and between 088dfec and 559b0ec.

📒 Files selected for processing (2)
  • python/cuopt/cuopt/routing/_serialize.py
  • python/cuopt/cuopt/tests/routing/test_serialize.py

Comment thread python/cuopt/cuopt/routing/_serialize.py Outdated
Comment thread python/cuopt/cuopt/routing/_serialize.py
Comment thread python/cuopt/cuopt/tests/routing/test_serialize.py
- to_host_problem: add parameter/return type hints and a full docstring
  (parameters, returns, KeyError behavior).
- Guard the 1:1 set_* fallback: a multi-argument set_* with no handler now
  raises instead of silently dropping its extra arguments; add a regression
  test for that path.
- Strengthen the export tests to assert the complete structures (matrix
  dtype/vehicle_type, both time-window arrays, capacity demand+capacity, and
  every field of both vehicle breaks including locations).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
@ramakrishnap-nv ramakrishnap-nv self-assigned this Jul 17, 2026
@ramakrishnap-nv ramakrishnap-nv added non-breaking Introduces a non-breaking change improvement Improves an existing functionality labels Jul 17, 2026
@ramakrishnap-nv
ramakrishnap-nv marked this pull request as ready for review July 17, 2026 19:12
@ramakrishnap-nv
ramakrishnap-nv requested a review from a team as a code owner July 17, 2026 19:12
@ramakrishnap-nv
ramakrishnap-nv requested a review from tmckayus July 17, 2026 19:12
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown

CI Test Summary

✅ All 22 test job(s) passed.

@ramakrishnap-nv

Copy link
Copy Markdown
Collaborator Author

/merge

@rapids-bot
rapids-bot Bot merged commit 51c07e8 into main Jul 20, 2026
155 of 157 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants