Fix hostdevice_vector::subspan
#13187
Merged
Conversation
This file contains 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
ttnghia
added
bug
davidwendt
approved these changes
Apr 20, 2023
PointKernel
reviewed
Apr 20, 2023
PointKernel
approved these changes
Apr 20, 2023
bdice
approved these changes
Apr 20, 2023
There was a problem hiding this comment.
Looks like a proper fix. Part of me wishes subspan was a free function rather than a method, so that hostdevice_vector would be closer in API to std::vector. If it were a method, I think it could be written for any container with hostdevice semantics (hostdevice_vector or hostdevice_span).
edit: I see there's a method std::span<T>::subspan, and this is based on that but applied to hostdevice_vector. This is taking the wrong lesson about container design from the STL, in my view, where a method could have been avoided in favor of a free function accepting any span-like object and returning a subspan (a non-owning view of partial extent).
|
/merge |
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.
This fixes a bug in
hostdevice_vector::subspanthat incorrectly constructs ahostdevice_spanwith the input parameters.In particular, instead of passing the
countparameter tohostdevice_spanconstructor, it (incorrectly) subtractsoffsetfromcount.