-
Notifications
You must be signed in to change notification settings - Fork 632
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
Non-materialized dft fields for adjoint calculations #1855
Conversation
Currently, the For example, assume we are looping over the adjoint field chunks, and we are currently on the I was thinking it may be worth it to create a modified version of |
Probably easier to just call |
As discussed in #1555, we'll probably need to ensure that loop_in_chunks only loops over owned points for this to work. |
Status update for @oskooi Currently, cases with complex fields are failing (including cylindrical coordinates). Also, the jax wrapper needs some love. But other than that, things look good. There is still some error introduced (hopefully) due to the bug within #1555. But the error is rather small. And not present when only a single chunk holds DFT fields. |
Co-authored-by: Steven G. Johnson <[email protected]>
I can't replicate the test failures on my machine. @oskooi can you try one more time on your machine? Note I've already bumped the tolerance, but am still seeing errors. |
With the most recent commit (9aa8296), there are two separate CI test failures:
On my local machine, I was only able to reproduce (1). |
This PR sometimes causes a segmentation fault in the adjoint run. An example for the
|
Before this PR, although there is no segmentation fault, the finite-difference and adjoint gradients are inconsistent in the above example. With the following code added to the end,
the results are printed as
|
After the PR, in addition to the above issue in the
Before this PR, the results are
After this PR, the results are
|
Closes #1797, #1832, #1861
TODO
dft_fields
allowing them to persist after a simulation is run