cuda.core: account for DLTensor.byte_offset in from_dlpack - #2594
Open
fedonman wants to merge 1 commit into
Open
cuda.core: account for DLTensor.byte_offset in from_dlpack#2594fedonman wants to merge 1 commit into
fedonman wants to merge 1 commit into
Conversation
DLPack places a tensor's first element at data + byte_offset, but view_as_dlpack set StridedMemoryView.ptr from data alone. A producer that reports the allocation base in data and expresses a slice as byte_offset therefore produced a view pointing byte_offset bytes before the tensor, with nothing raised. The offset was also lost permanently on a round-trip, because the __dlpack__ re-export writes ptr back out as data with byte_offset = 0. The capsule-consuming importer in the same module already folds byte_offset in, so the two import paths disagreed about the same capsule. This makes view_as_dlpack match it. Closes NVIDIA#2592 Signed-off-by: Vyron Vasileiadis <hi@fedonman.com>
Contributor
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.
Description
closes #2592
DLPack places a tensor's first element at
data + byte_offset.view_as_dlpack, which backsStridedMemoryView.from_dlpackandfrom_any_interface, setptrfromdataalone, so a producer that reports the allocation base indataand expresses a slice asbyte_offsetproduced a view pointingbyte_offsetbytes before the tensor, with nothing raised. The capsule-consuming importer in the same module,_smv_from_dlpack_capsule, already foldsbyte_offsetin, so the two import paths disagreed about the same capsule. This makesview_as_dlpackmatch it, and that one line is the whole production change.Correcting
ptralso fixes the round-trip. The__dlpack__re-export writesptrback out asdatawithbyte_offset = 0, so before this change a non-zero offset was dropped permanently on the way back out.The new test exercises both the versioned and the unversioned capsule branches. It views the first 8 elements of a 9-element array, then re-describes the same tensor as
src[1:9]by writingbyte_offsetalone, so a regression fails an assertion rather than reading out of bounds.shapeandstridesshare a single producer-owned allocation, so the test does not touch them.A release note is added under
Fixes and enhancementsincuda_core/docs/source/release/1.2.0-notes.rst; the affected behavior has been present since 0.1.0.Verification
The new test, both capsule flavours:
The same test with the one-line change reverted:
The full
cuda_coresuite:Checklist