Open divyegala opened 10 months ago
The mdbuffer PR already includes managed and pinned mdarray support. We needed a couple workarounds for the missing RMM functionality you mentioned, but it's there. Depending on whether the mdbuffer work gets deprioritized again or not, I can split it off into a separate PR as I did for the mdspan copy utility. We can follow up with cleaner versions once those changes are in RMM.
@wphicks did you include new verbiage for managed and pinned mdarray support in the mdbuffer PR like this issue mentions or are they a part of host_mdarray
?
Please keep me in the loop when mdbuffer is higher priority on your task-list. I'd like to help you avoid any workarounds by adding/modifying code upstream to RMM
I added new verbiage. We now have managed_mdarray
and pinned_mdarray
(though I'm not married to those names). I'll go ahead and post the mdbuffer draft soon and link to the specific implementation details. It would be awesome if we could avoid the current workarounds for sure! I'll touch base with you on details ASAP.
Linking disscussion https://github.com/rapidsai/raft/pull/1896#discussion_r1387814866
Working on https://github.com/rapidsai/raft/pull/1748, I realized the need to introduce types that allow for memory access from host and device both, as more algorithms need to support allocating either
managed
orpinned
memory.How I came up with an RAII solution to allocating pinned host memory:
RMM
Issue with using an RMM based allocator (
rmm/mr/host/pinned_memory_resource.hpp
): -thrust
allocates elements by keeping track of a template typeT
for the element to be allocated whereas RMM allocates bytes directlythrust::device_malloc_allocator
rmm::device_uvector
only allows an allocator derived from adevice_memory_resource
Fix
thrust_allocator_adaptor.h
to allowrmm::thrust_allocator
for inheriting from any arbitrarythrust::allocator
and using it withthrust::host_vector
Consequences
thrust::host_vector
when RAFT should be able to natively support these solutionsRAFT
Why a
universal_*
vocabulary makes the most sensestd::allocator<ElementType
raft::host_mdarray
we ourselves usestd::allocator<ElementType>
for allocations withstd::vector
under the hoodFix
pinned_memory_allocator
andmanaged_memory_allocator
to RAFT does not make much sense in terms of using simple factories to create RAFT typespinned_container_policy
andmanaged_container_policy
inraft/core/universal_container_policy.hpp
and creating equivalentuniversal_mdspan/mdarray
types gives us the most flexibility and removes confusion for users and devs of having to work with allocators and memory resourcesConsequences
raft::make_pinned_host_mdarray
andraft::make_managed_mdarray
inuniversal_mdarray.hpp
universal_mdarray::view()
producesuniversal_mdspan
raft::make_host_mdspan(universal_mdspan)
andraft::make_device_mdspan(universal_mdspan)
to let dev allow the type to go through generic host/device paths when neededConclusion We'll need updates to both RMM and RAFT as discussed above, but it will ultimately end up giving us more flexibility while letting us define an increasingly important type.