Provide memory_type enum #984
Conversation
Provide an enum class used to distinguish among various memory types. This allows decoupling of the concept of memory type from a specific mdspan accessor policy and ensures that templates receive only a logically-consistent memory type.
wphicks
added
feature request
Co-authored-by: Rajesh Gandham <[email protected]>
As a follow-up to rapidsai#984 which introduces a memory_type enum, this PR introduces a device_type enum with similar justifications.
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| #pragma once |
There was a problem hiding this comment.
I really like consolidating the different memory variations into it's own enum. I'm wondering if we should integrate / consolidate this further into the raft::host_device_accessor here? or maybe there's no value in doing that quite yet?
There was a problem hiding this comment.
I think it's certainly worth doing as a follow-up.
|
I know we are all moving fast to try to get things merged in and get boxes checked but I'd like to make sure we don't lose sight of these types of things and end up with multiple strategies for handling different memory types that are somewhat similar but not quite the same across raft and consuming projects. Unless you think it would be possible to consolidate this with the accessor immediately as a follow-up, can you create a GitHub issue for just to keep it on our radar? |
|
Yep! Just posted: #994 |
There was a problem hiding this comment.
The PR looks very good, but I'd much prefer if we consolidate this type everywhere (mdspan's accessors included). It's not a breaking change to do that since that is not directly exposed to the user, so it's nicer to consolidate new types in one go and not have disparity that adds on tech debt. I imagine the PR will still remain lean.
Also update style
|
@gpucibot merge |
This PR introduces an enum to specify a memory type (e.g. host, device, managed...). This allows us to provide a template parameter which always indicates a valid memory type, which is extensible for possible future memory types, and which is less verbose than alternatives.
The most serious shortcoming of existing alternatives is the possibility of indicating invalid memory states. E.g. by templating on
is_deviceandis_host, we introduce the possible stateis_host=false, is_device=false.