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

NLPP derivatives in optimization need SoA #2083

Closed
jtkrogel opened this issue Nov 6, 2019 · 6 comments · Fixed by #2179
Closed

NLPP derivatives in optimization need SoA #2083

jtkrogel opened this issue Nov 6, 2019 · 6 comments · Fixed by #2179
Labels

Comments

@jtkrogel
Copy link
Contributor

jtkrogel commented Nov 6, 2019

Currently only supported in AoS.

@jtkrogel
Copy link
Contributor Author

It turns out that NLPP derivatives are sometimes crucial for correct optimization. Below is a comparison plot of the change in the total energy for Li2 during Jastrow optimization with and without the NLPP derivatives on.

energy_vs_series

For work on forces, we will have to use the AoS code for optimization until SoA is fixed for this.

@jtkrogel
Copy link
Contributor Author

Who is looking into fixing this issue?

@jtkrogel jtkrogel added this to the v4.0.0 Release milestone Nov 22, 2019
@jtkrogel jtkrogel added the bug label Nov 22, 2019
@ye-luo
Copy link
Contributor

ye-luo commented Nov 22, 2019

OK. I will take care of it next week.

@prckent
Copy link
Contributor

prckent commented Dec 3, 2019

Still investigating, but I think another user might have been caught by this for a heavier system.
Since SoA is the default, it means that new users will have worse wavefunction optimization than they should. Once implemented, I think we should make sure that NLPP derivatives are on by default in all examples, change the code default etc. Better to be slower but reliable.

@jtkrogel
Copy link
Contributor Author

jtkrogel commented Dec 3, 2019

I agree 100% that these should be on by default.

@ye-luo
Copy link
Contributor

ye-luo commented Dec 4, 2019

  1. This feature should be default.
  2. implementing optimized virtual move code path is necessary to have painless 1.

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

Successfully merging a pull request may close this issue.

3 participants