refactor(qdp): rename validate_tensor to validate_tensor_cpu and drop redundant check - #1415
Merged
Merged
Conversation
… redundant check validate_tensor() re-checked is_pytorch_tensor() even though its only caller (QdpEngine::encode -> encode_from_pytorch) already guarantees the object is a torch.Tensor before dispatching to the CPU path. Remove the redundant check and rename the helper to validate_tensor_cpu to reflect that it only validates device placement -- which is also what rejects the non-CPU backends (CUDA/MPS/XLA/HPU). No behavior change.
Contributor
|
cc @400Ping |
400Ping
approved these changes
Jul 3, 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
3 participants
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.
Related Issues
Closes #907
Changes
Why
Follow-up to the review thread on #881.
validate_tensor()re-checkedis_pytorch_tensor(tensor)even though its only caller alreadyguarantees the object is a
torch.Tensor:validate_tensorhas a single call site:QdpEngine::encode_from_pytorch(
engine.rs:209).encode_from_pytorchis only reached from theencode()dispatcher(
engine.rs:125), and that branch is already gated byis_pytorch_tensor(data)?atengine.rs:119.So the inner check was dead weight on the real execution path. The
function's actual job is validating device placement — the
device.type != "cpu"guard, which is what already rejects non-CPUbackends (CUDA/MPS/XLA/HPU). The old name
validate_tensordidn't conveythat, hence the rename suggested in the issue.
How
is_pytorch_tensorcheck (and its now-dead"Object is not a PyTorch Tensor"error arm) from the helper.validate_tensor→validate_tensor_cpu, with a doc commentstating the caller-side contract (caller has already confirmed it is a
torch.Tensor) and that it rejects non-CPU backends.engine.rs.No behavior change on any supported path. Verified with
cargo check(--cfg qdp_no_cuda) andcargo fmt --check.Checklist
validate_tensor_cpu