- 
                Notifications
    You must be signed in to change notification settings 
- Fork 9
Introduce safe transpose #339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
812d437    to
    8ed52f0      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| /// \tparam iType The index type used for the view | ||
| /// \tparam ArgBoundsCheck The bounds check type (default is Off) | ||
| template <typename ExecutionSpace, typename InViewType, typename OutViewType, | ||
| typename iType> | ||
| typename iType, typename ArgBoundsCheck = BoundsCheck::Off> | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit unrelated with this PR but I would take advantage of it to change iType to IndexType for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review
It's a bit unrelated with this PR but I would take advantage of it to change iType to IndexType for consistency.
I guess I use iType in multiple functions
Can I rename them in another PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I was proposing to do this one now since you are touching it, but if you want to change them all it would be better to do it in a dedicated PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I will merge this first and make a dedicated PR
This PR aims at introducing bound check option to transpose function.
BoundsCheckoption to Transpose functor.transposefunction to performdeep_copyif transpose is not needed (Done in Copy if map is identical in transpose helper #345)transposefunction to work on Views with different Layout (Done in Make transpose helper to work on Views with different layout #344)transposefunction to work on integral typeis_transpose_neededfunction to work on integral type (Done in Allow is_transpose_needed to work on std::size_t based array #350)bounds_check. Needs to improve the case withbounds_checkto work on differentViewshapes