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.

Sign up for GitHub

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

Jump to bottom

Hellmann-Feynman stresses via ForwardDiff and custom rules #476

Merged
merged 52 commits into from
Jul 15, 2021
Merged

Hellmann-Feynman stresses via ForwardDiff and custom rules #476

merged 52 commits into from
Jul 15, 2021

Conversation

niklasschmitz
Copy link
Collaborator

@niklasschmitz niklasschmitz commented Jul 8, 2021 •
edited
Loading

This PR implements experimental support for Hellmann-Feynman stresses via ForwardDiff.
Coming from niklasschmitz#2 and #443

This does not include yet:

  • external code like libxc

TODO list

  • re-enable lattice cond check
  • symmetries
  • safer type promotion in compute_density
  • solve DummyInplace code duplication with requires
  • use silicon numbers from test/testcases.jl

@niklasschmitz
add kinetic stress autodiff example
d442e91
@niklasschmitz
add stress-total error messages of FD, RD, Zygote
2074095
@niklasschmitz
add stress-total scalar test
cafed13
@niklasschmitz
disable lattice cond check temporarily
38db38f
@niklasschmitz
add model_atomic_debug with Kinetic() only
797bf34
@niklasschmitz
stacktrace: no next_working_fft_size for Duals
223dc24
@niklasschmitz
stress-kinetic with generic fft forwarddiff works
abe7c7b
@niklasschmitz
move model term selection to call site
4861677
@niklasschmitz
generic fft: Kinetic, AtomicLocal, Ewald, Psp work
4bea529
@niklasschmitz
direct return
30dbc98
@niklasschmitz
use make_basis in stress-total
a5660f8
@niklasschmitz
rename stress-forward-genericlinearalgebra
746aad6
@niklasschmitz
stack trace: no next_working_fft_size for Dual
cdad8eb
@niklasschmitz
stack trace: no build_fft_plans for Dual
324918f
@niklasschmitz
reanme stress-forward-genericfft
9e772a5
@niklasschmitz
add generic-fft stress btimes
e7c4105
@niklasschmitz
add (a few) FFTW ForwardDiff rules
5512526
@niklasschmitz
stack trace: no mul!(...Dual, ...FFTW.cFFTWPlan,.)
1a5b54c
@niklasschmitz
fwddiff FFTW: Kinetic, AtomicLocal, Ewald, Psp
42cb076
@niklasschmitz
add fwddiff FFTW btimes
791a5a8
@niklasschmitz
add description header
320ce60
@niklasschmitz
update generic fft stress values
57331b0
@niklasschmitz
del forward-genericfft in favor of seperate branch
3da7d29
@niklasschmitz
update stack traces for Zygote, ReverseDiff
7a0dcf3
@niklasschmitz
stack trace: one carbon
e763015
@niklasschmitz
stack trace: AtomicNonlocal NaN ForwardDiff
34b5129
@niklasschmitz
fix AtomicNonlocal NaN
5cf5a2c
@niklasschmitz
fix fft normalization & dual-scaled plans
eb89f36
@niklasschmitz
add ForwardDiff norm of SVector workaround
cb1b968
@niklasschmitz
add total stress result of ForwardDiff
6eafdef
antoine-levitt
antoine-levitt reviewed Jul 8, 2021
View reviewed changes
Copy link
Member

@antoine-levitt antoine-levitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonderful! Can we just merge as is or is the type piracy wrt ForwardDiff a problem?

test/stresses.jl Outdated Show resolved Hide resolved
Project.toml Outdated Show resolved Hide resolved
mfherbst
mfherbst reviewed Jul 9, 2021
View reviewed changes
src/DFTK.jl Outdated Show resolved Hide resolved
src/densities.jl Outdated Show resolved Hide resolved
src/densities.jl Outdated Show resolved Hide resolved
src/densities.jl Outdated Show resolved Hide resolved
test/stresses.jl Outdated Show resolved Hide resolved
src/workarounds/forwarddiff_rules.jl Show resolved Hide resolved
src/workarounds/forwarddiff_rules.jl Outdated Show resolved Hide resolved
src/workarounds/forwarddiff_rules.jl Outdated Show resolved Hide resolved
src/workarounds/forwarddiff_rules.jl Outdated Show resolved Hide resolved
src/workarounds/forwarddiff_rules.jl Outdated Show resolved Hide resolved
@niklasschmitz niklasschmitz marked this pull request as draft July 13, 2021 08:34
antoine-levitt
antoine-levitt reviewed Jul 15, 2021
View reviewed changes
src/densities.jl Outdated Show resolved Hide resolved
@niklasschmitz niklasschmitz marked this pull request as ready for review July 15, 2021 11:43
mfherbst
mfherbst reviewed Jul 15, 2021
View reviewed changes
src/Model.jl Outdated Show resolved Hide resolved
mfherbst
mfherbst reviewed Jul 15, 2021
View reviewed changes
src/Model.jl Outdated Show resolved Hide resolved
@mfherbst mfherbst enabled auto-merge (squash) July 15, 2021 12:09
@mfherbst mfherbst merged commit c6da218 into JuliaMolSim:master Jul 15, 2021
This was referenced Jul 15, 2021
Closed
Closed
@niklasschmitz
Copy link
Collaborator Author

niklasschmitz commented Jul 15, 2021 •
edited
Loading

Thanks for the review @mfherbst @antoine-levitt !

@antoine-levitt
Copy link
Member

Thanks for the great work!

This was referenced Aug 20, 2021
Closed
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfherbst mfherbst mfherbst left review comments

@antoine-levitt antoine-levitt antoine-levitt left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@niklasschmitz @antoine-levitt @mfherbst