-
-
Notifications
You must be signed in to change notification settings - Fork 233
Fix #914: rewrite integer powers to multiplication on GPU #1021
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
base: master
Are you sure you want to change the base?
Conversation
Add GPUUtils module that rewrites integer powers (u^2, u^3, etc) to multiplication chains (u*u, u*u*u) before GPU kernel generation. This preserves Symbolics derivative rules and avoids AD issues. Only applies when init_params are on GPU (via AbstractGPUDevice). Tests cover symbolic transformations, device detection, and the original failing cases (u^2, Dx(u^3)) on CUDA. Non-CUDA systems skip GPU tests.
Use SymbolicUtils.term for multiplication construction
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.
this isn't in the runtests so it won't be ran.
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.
Dr Chris, Thank you for pointing this out. I might be missing something here.
I added them as @testItems and was relying on ReTestItems to pick them up via the :cuda tags.
If you’d prefer them to be explicitly included in runtests.jl or moved alongside the existing CUDA PDE tests, I’d be happy to adjust.
src/gpu_utils.jl
Outdated
| Transform integer power operations into explicit multiplication chains | ||
| compatible with symbolic differentiation. | ||
|
|
||
| This function rewrites expressions of the form `u^n` (where `n` is a positive | ||
| integer) into equivalent multiplication expressions `u * u * ... * u` (n times). | ||
| This transformation enables automatic differentiation through the Symbolics.jl | ||
| chain rule without requiring special-cased derivative rules for power operations. |
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.
This doesn't seem like it would generate more efficient code?
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.
You’re right. yes
I did that to avoid NaNs we were seeing on GPU backward passes for expressions like u(x)^2 and Dx(u(x)^3) (issue #914).
I'll update the comment to make the intent clearer, but would you prefer to handle this error a different way?
Added CUDA import to test items
Fix missing imports in gpu_nonlinear_tests.jl Documentation: clarify purpose of transform_power_ops
Added GPUUtils module that rewrites integer powers (u^2, u^3, etc) to multiplication chains (uu, uu*u) before GPU kernel generation.
Tests cover symbolic transformations, device detection, and the original failing cases (u^2, Dx(u^3)) on CUDA. Non-CUDA systems skip GPU tests.
Note: Developed on Mac M1. CPU tests pass. Please run CUDA tests.