Skip to content
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

Replace custom CuPy implementation of SciPy APIs #88

Open
ziw-liu opened this issue Nov 4, 2022 · 3 comments
Open

Replace custom CuPy implementation of SciPy APIs #88

ziw-liu opened this issue Nov 4, 2022 · 3 comments
Labels
enhancement New feature or request

Comments

@ziw-liu
Copy link
Contributor

ziw-liu commented Nov 4, 2022

waveorder.util.uniform_filter_2D currently uses a custom set of numerical operations to reimplement scipy.ndimage.uniform_filter on GPU.

This should be replaced by calling the CuPy API directly.

@ziw-liu ziw-liu added the enhancement New feature or request label Nov 4, 2022
@mattersoflight
Copy link
Member

@ziw-liu @talonchandler This code is likely used in PTI reconstruction - we need to filter permittivity tensor components individually for averaging. It looks like all the scalar simulation and reconstruction algorithms are now implemented with pytorch. Let's touch base in person on the status of vector simulation/reconstruction code, and how to get to pytorch implementation.

@ziw-liu
Copy link
Contributor Author

ziw-liu commented Jul 25, 2023

We would also need to make a plan for GPU support with torch. For now CuPy is still referenced in README on main but it only works for v1.0.0.

@talonchandler
Copy link
Collaborator

Thanks @ziw-liu @mattersoflight.

I'm going to keep this issue open, because I still think we should migrate from the custom waveorder.util.uniform_filter_2D to a scipy implementation. @mattersoflight is correct that this function is used by the PTI reconstruction, so this migration can happen alongside the PTI migration to torch, see #142.

I've also opened a new issue for discussion of GPU support #144. For now, the README on main is still accurate since there are still PTI parts of the code that utilize CuPy. I think we can remove CuPy (and update the README) when everything has moved to torch, see #143.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants