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

AD for forces #868

Closed
wants to merge 1 commit into from
Closed

AD for forces #868

wants to merge 1 commit into from

Conversation

epolack
Copy link
Collaborator

@epolack epolack commented Jul 24, 2023

Precompiling is the same (but slow ~100s, I do not understand why; I had this also in #853, then it reverse to normal timings without any changes from my part…).

On a simple silicon supercell of size 2×2×2 the cold and hot timings are similar:
master: 22s then 6.29s
branch: 24.2s then 6.57s

So if the change seems fine for you, I will test this on more robust test cases (ideas are welcome).

@antoine-levitt
Copy link
Member

Not too sure about this. Generally, trying to add more AD introduces more headaches down the line. Esp. for forces it's probably possible to use adjoint tricks, which I don't think we do currently but would be probably easier to do with explicit derivatives. Also, here you have to do a lot more FFTs. Is there a motivation to this?

(as an aside, we have the pattern "build a superposition of radial things" quite a lot, we should factor it out eventually)

@epolack
Copy link
Collaborator Author

epolack commented Jul 24, 2023

Flexibility for phonons, as I need also ∫δρδV and ∫ρδ²V. From a comment you made at some point. I used to compute them by hand and you asked if using AD would not simplify the code.

@mfherbst
Copy link
Member

I'm also not too sure about this as we don't actually loose that much code. Also not sure how well this plays with GPU. I think we should have a proper think about how to do this, if we really want to.

@epolack epolack marked this pull request as draft October 3, 2023 15:25
@epolack epolack closed this Dec 12, 2023
@epolack epolack deleted the loc_ad branch December 12, 2023 15:11
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

Successfully merging this pull request may close these issues.

3 participants