API: Device API v2. Removed old API. - #1342
Merged
Merged
Conversation
rakhmets
requested review from
a team,
brminich,
gleon99 and
yosefe
as code owners
February 18, 2026 13:41
|
👋 Hi rakhmets! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
rakhmets
force-pushed
the
topic/rm-device-api-v1
branch
from
February 19, 2026 10:08
230c16c to
22ca3cd
Compare
Signed-off-by: Raul Akhmetshin <rakhmetshin@nvidia.com>
Signed-off-by: Raul Akhmetshin <rakhmetshin@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/plugins/ucx/mem_list.cpp (1)
184-184: Previous suggestion applied correctly.The
constexpr std::string_viewandstd::string{error_message}construction at the throw sites match what was previously suggested. The anonymous namespace wrapping is also consistent with the project's style guidelines (anonymous namespaces preferred overstatic).Also applies to: 190-190, 195-195
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/ucx/mem_list.cpp` at line 184, The constexpr std::string_view error_message{"UCX GPU device API is not supported"} should be defined in the anonymous namespace and reused at all throw sites in mem_list.cpp (replace ad-hoc string constructions with std::string{error_message} where the exception is thrown); ensure the variable name error_message is used consistently at each throw site so the same message and type are reused (this same change should be applied to the other throw locations that previously used raw string literals).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/device/ep/csrc/kernels/nixl_ep.cu`:
- Around line 262-263: Change the inconsistent cast of dst_rank when
constructing nixlMemViewElem: replace the (unsigned) cast with (size_t) so the
index field receives a size_t like other call sites; update the construction of
nixlMemViewElem (using nixl_ctx.remote_mvh, (size_t)dst_rank,
nixl_ctx.offset_get(dst_ptr)) so subsequent EP_DEVICE_ASSERT and nixlAtomicAdd
calls (with num_tokens_sent and dst_expert_local_idx) use the correct type.
In `@src/api/gpu/ucx/nixl_device.cuh`:
- Around line 38-49: Add an explicit send_push constant and make to_ucp_flags
validate incoming bits: in namespace nixl_gpu_flags declare constexpr uint64_t
send_push = 0; and update to_ucp_flags(uint64_t flags) to first assert that
(flags & ~known_mask) == 0 where known_mask is the bitwise OR of all known flags
(defer | send_push), then map known bits to UCP flags (keep existing logic for
defer -> toggle UCP_DEVICE_FLAG_NODELAY) and fail fast (e.g., assert or
static/runtime check) if unknown bits are present so unrecognised flags are not
silently dropped.
---
Duplicate comments:
In `@src/plugins/ucx/mem_list.cpp`:
- Line 184: The constexpr std::string_view error_message{"UCX GPU device API is
not supported"} should be defined in the anonymous namespace and reused at all
throw sites in mem_list.cpp (replace ad-hoc string constructions with
std::string{error_message} where the exception is thrown); ensure the variable
name error_message is used consistently at each throw site so the same message
and type are reused (this same change should be applied to the other throw
locations that previously used raw string literals).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
examples/device/ep/csrc/kernels/nixl_ep.cusrc/api/gpu/ucx/nixl_device.cuhsrc/plugins/ucx/mem_list.cpptest/gtest/device_api/single_write_test.cu
Signed-off-by: Raul Akhmetshin <rakhmetshin@nvidia.com>
Signed-off-by: Raul Akhmetshin <rakhmetshin@nvidia.com>
Contributor
Author
|
/build |
itayalroy
previously approved these changes
Feb 26, 2026
itayalroy
left a comment
Contributor
There was a problem hiding this comment.
nixl_ep changes look good
Signed-off-by: Raul Akhmetshin <74596089+rakhmets@users.noreply.github.com>
Contributor
Author
|
/build |
ofirfarjun7
reviewed
Feb 26, 2026
yosefe
approved these changes
Feb 27, 2026
This was referenced Mar 3, 2026
1 task
This was referenced Jul 14, 2026
This was referenced Aug 24, 2026
This was referenced Sep 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Deleted the previous version of the Device API.
Renamed
nixlMemViewElementtonixlMemViewElem.Replaced
NO_DELAYflag bydefer, making withoutdeferdefault.Summary by CodeRabbit
Refactor
Breaking Changes
Tests