-
-
Notifications
You must be signed in to change notification settings - Fork 83
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
Should nccl::broadcast
take a Option<&T>
instead of an &Option<T>
?
#290
Comments
Hmm yeah actually I agree with you. At first I thought Option provided a way to do this, but its the reverse - Option::as_ref "Converts from |
I think using the CudaViews is a fairly okay work around for this (probably why it's usable in the first place). Creating a view is very cheap and you can reuse them as many times as you want. Unless I'm missing something. |
yeah this is a workaround, but only possible since the last version while this interface was there much longer, just wanted to raise this as a suggestion |
In
cudarc/src/nccl/safe.rs
Lines 242 to 245 in 886d6d2
&Option<T>
, which involves wrapping theT
, which is something like aCudaSlice
in an Option to pass it in there, making re-use of the sendbuffer later on less ergonomic, since you need to get it back from theOption
. The other way around would make this easier, but IDK if there are some other drawbacks I'm not seeing ATM?The text was updated successfully, but these errors were encountered: