search

Rust-GPU / Rust-CUDA

Ecosystem of libraries and tools for writing and executing fast GPU code fully in Rust.
Apache License 2.0
3.17k stars 120 forks source link

Implement DeviceCopy for Complex numbers #10

Closed sebcrozet closed 2 years ago

sebcrozet commented 3 years ago

This PR adds an optional implementation of DeviceCopy for Complex numbers. This can be very valuable for scientific computing. The num_complex::Complex type is already Copy and #[repr(C)].

RDambrosio016 commented 3 years ago

I'm kind of wondering if this is the wrong approach, it might be better to instead PR a CUDA feature to num to make it derive DeviceCopy. Just so we don't end up with hundreds of features.

RDambrosio016 commented 3 years ago

The plan currently is to actually move DeviceCopy out of cust and into cust_core which will build on gpu targets, that way you don't need tons of cfg's for structs. So i think i will hold off on merging this until then, that way we can decide if we want this in cust_core or in num-complex. Im personally for the latter considering how many math crates could be useful for the GPU, but i am not sure.

sebcrozet commented 3 years ago

@RDambrosio016 I think that @cuviper makes a very good point: since Rust-CUDA will likely evolve a lot in the near future, it is best to have the cust (or cust_core) depend on num-complex instead of the other way round. Then we could revisit this decision in a few years once Rust-CUDA becomes more stable.

The core issue with having num-complex depend on cust (or cust_core) is that each minor version bump of cust (which we can expect to happen fairly frequently in the near future) would imply a minor version bump of num-complex, which in turns implies a minor version bump of the whole ecosystem relying directly or indirectly on num-complex (even if they don’t care about CUDA).

Having cust depend on num-complex on the other hand would have little consequences on the ecosystem versioning, and people not caring about CUDA support won’t be affected at all. The maintainance burden added to cust/cust_core is minimal considering that num-complex is already fairly stable it this point (with only 3 minor version bumps during the past two years).

RDambrosio016 commented 3 years ago

Hmm thats a good point, tho we will need to move this to cust_core later