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

Refactor to use complex numbers #20

Open
astanziola opened this issue Apr 4, 2022 · 2 comments
Open

Refactor to use complex numbers #20

astanziola opened this issue Apr 4, 2022 · 2 comments

Comments

@astanziola
Copy link
Member

According to this repository for Fourier Neural Operators, they've made their code 30% faster by switching to native complex representation in PyTorch, which wasn't available when this work was published.

Is it worth refactoring the code to use complex multiplications? On one hand, it would be nice to have a faster codebase, on the other hand this repo makes sense for research but not really for production, so perhaps one doesn't really care about speed.

@sio13 @SonyPony Any suggestion? (Is it about time to just move everything to JAX? 😄 )

@SonyPony
Copy link
Contributor

SonyPony commented Apr 4, 2022

Well, FNO has way more complex multiplications than helmnet. However, even ~10% speedup would be nice. At least the training won't take a whole day 😛.

If the only part using complex operations is the residual computation, the refactoring should be quite straightforward.

JAX would be cool 😄, but I don't know whether it's worth moving the whole project to JAX.

@astanziola
Copy link
Member Author

I think it is just that, probably only complex_mul needs to be removed and its call refactored.

Although I'm not totally sure that gradients are totally the same, see this note about autograd with complex numbers. Probably worth to add a quick test to be sure.

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

No branches or pull requests

2 participants