-
Notifications
You must be signed in to change notification settings - Fork 15
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
fix(cuda): fix multi-GPU implementation when there are few inputs #149
Conversation
@slab-ci gpu_test |
@slab-ci cpu_test |
@slab-ci cpu_test |
concrete-core/src/backends/cuda/implementation/engines/cuda_engine/mod.rs
Outdated
Show resolved
Hide resolved
...ckends/cuda/implementation/engines/cuda_engine/lwe_ciphertext_vector_discarding_bootstrap.rs
Outdated
Show resolved
Hide resolved
918f2e5
to
f8ef830
Compare
@slab-ci gpu_test |
@slab-ci cpu_test |
f8ef830
to
c7da817
Compare
@slab-ci gpu_test |
let data_per_gpu = input.glwe_dimension().to_glwe_size().0 | ||
* input.glwe_dimension().to_glwe_size().0 | ||
* input.input_lwe_dimension().0 | ||
* input.decomposition_level_count().0 | ||
* input.polynomial_size().0; | ||
let size = data_per_gpu as u64 * std::mem::size_of::<u32>() as u64; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there already is the need for the compiler in practice https://github.com/zama-ai/concrete-core-internal/issues/188
Impressive work refactoring the CUDA backend, @agnesLeroy ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks good 🙂 great effort !
concrete-core/src/backends/cuda/private/crypto/bootstrap/mod.rs
Outdated
Show resolved
Hide resolved
concrete-core/src/backends/cuda/private/crypto/bootstrap/mod.rs
Outdated
Show resolved
Hide resolved
c7da817
to
5744513
Compare
@slab-ci gpu_test |
5744513
to
a03f4be
Compare
@slab-ci gpu_test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few things still (sorry)
...ore/src/backends/cuda/implementation/engines/cuda_engine/lwe_ciphertext_vector_conversion.rs
Outdated
Show resolved
Hide resolved
concrete-core/src/backends/cuda/private/crypto/bootstrap/mod.rs
Outdated
Show resolved
Hide resolved
a03f4be
to
42315ac
Compare
- also, change some types so that it feels more natural to use them - refactor the whole backend to reduce the probability of bugs
42315ac
to
dfeaeba
Compare
@slab-ci gpu_test |
@IceTDrinker normally I took all the comments into account, GPU tests are green! 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good as far as I can tell !
For the twiddle thing can it be something that the fft does by itself ?
Maybe, we need to do some benchmarking to check the effect on performance. |
Resolves: https://github.com/zama-ai/concrete-core-internal/issues/382 and https://github.com/zama-ai/concrete-core-internal/issues/389
Description
Checklist
(Use '[x]' to check the checkboxes, or submit the PR and then click the checkboxes)