-
Notifications
You must be signed in to change notification settings - Fork 356
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
Add decompress module for prototype cupy backend #4962
base: master
Are you sure you want to change the base?
Add decompress module for prototype cupy backend #4962
Conversation
Testingmismatches:
This means that the compressed waveform implementation on CPU and CuPy is exactly the same |
d0ff667
to
99c3770
Compare
(@GarethCabournDavies You will need to wait for #4962 to be merged before sanity tests could pass here, or at least you'll need fixes in there (like for documentation build). I'll hold on reviewing this until #4962 is merged) |
99c3770
to
61d9f5d
Compare
(nb,), (nt,), | ||
(g_out, df, hlen, flow, fmax, texlen, freqs_gpu, amps_gpu, phases_gpu, lower, upper) | ||
) | ||
cp.cuda.runtime.deviceSynchronize() |
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.
Is this synchronize needed?
# Note that imin and start_index are ignored in the GPU code; they are only | ||
# needed for CPU. | ||
if output.precision == 'double': | ||
raise NotImplementedError("Double precision linear interpolation not currently supported on CUDA scheme") |
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.
Can't this be easily supported now with dynamic types in Cupy?
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.
Some minor comments, but I'd be happy to merge this once the tests pass.
For the CuPy backend, we need the decompress module translated; this is that translation
Standard information about the request
This is a new feature
This change affects the offline search, as I don't think any others (use compressed waveforms)
This change changes scientific output
This change follows style guidelines (See e.g. PEP8), has been proposed using the contribution guidelines
This change will require additional dependencies (CuPy in the new backend)
Contents
This is effectively a copy of the decompress_cuda module, but using cupy rather than pycuda
Links to any issues or associated PRs
#4952
Testing performed
Comparison of waveforms with/without compression and from CPU/CuPy: