Separating mdspan/mdarray infra into host_* and device_* variants #810
Conversation
cjnolet
added
improvement
cjnolet
added
breaking
|
@divyegala I'm on the fence about whether the But, for example, if we start supplying more complex |
| auto flatten(mdspan_type mds) | ||
| { | ||
| RAFT_EXPECTS(mds.is_exhaustive(), "Input must be contiguous."); |
There was a problem hiding this comment.
Please note that is_exhaustive() only means that the storage is contiguous. The range of the layout mapping need not necessarily be monotonically increasing (it could be backwards, for example).
There was a problem hiding this comment.
Can you provide an example of your suggested way to force this to be row or col major? (Ideally we could do this at compile time).
There was a problem hiding this comment.
Here's a way to check if an mdspan is row or column major.
template<class ElementType, class Extents, class Layout, class Accessor>
constexpr bool is_row_or_column_major(mdspan<ElementType, Extents, Layout, Accessor> /* m */ )
{
return false;
}
template<class ElementType, class Extents, class Accessor>
constexpr bool is_row_or_column_major(mdspan<ElementType, Extents, layout_left, Accessor> /* m */ )
{
return true;
}
template<class ElementType, class Extents, class Accessor>
constexpr bool is_row_or_column_major(mdspan<ElementType, Extents, layout_right, Accessor> /* m */ )
{
return true;
}
template<class ElementType, class Extents, class Accessor>
constexpr bool is_row_or_column_major(mdspan<ElementType, Extents, layout_stride, Accessor> m)
{
return m.is_exhaustive();
}(Relevant issue that you posted; thank you! #819 )
|
@mhoemmen, thank you for this thorough review! I've implemented most of your suggestions. I wasn't able to get this suggestion or this suggestion building, though most of the changes in this PR are just moving stuff around so I think we can continue to make things more clean and correct in follow-on PRs before release 22.10 is ready. Would you be interested in opening a PR w/ those changes? |
|
rerun tests |
There was a problem hiding this comment.
Love this! Had two questions on details, but this is a huge step forward for making use of mdspan in the FIL backend. I was not able to absolutely confirm that there were no gotchas with the CPU-only compilation, but everything looks good as best I can tell without finishing a version of the FIL code that makes use of this.
|
@gpucibot merge |
…ce_span` (#2679) This depends on rapidsai/raft#810. Closes #2322 Authors: - Corey J. Nolet (https://github.com/cjnolet) Approvers: - Seunghwa Kang (https://github.com/seunghwak) - Chuck Hastings (https://github.com/ChuckHastings) - Rick Ratzel (https://github.com/rlratzel) - Ray Douglass (https://github.com/raydouglass) - Mark Sadang (https://github.com/msadang) URL: #2679
This is a breaking change as it provides users with a more granular set of headers to import
hostseparately fromdeviceandmanagedversions. It also separates the headers formdspanandmdarray.As an example, the following public headers can now be imported individually:
cc @rg20 @afender @akifcorduk for awareness.
Closes #806