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

Fix NLPP derivative in SoA #2179

Merged
merged 6 commits into from
Jan 3, 2020
Merged

Conversation

ye-luo
Copy link
Contributor

@ye-luo ye-luo commented Jan 1, 2020

Needs #2177 merged in first.
Close #2083.

I mostly relied on unit tests. The full simulation case I tried didn't make a significant difference using or not using NLPP derivatives.
@jtkrogel Could you check your Li2 case?

@ye-luo ye-luo requested a review from jtkrogel January 1, 2020 13:24
@tiihonej
Copy link
Contributor

tiihonej commented Jan 2, 2020

After running similar I can reproduce the behavior in #2083, only this time I can use both SoA and AoS builds.

Li2_soa_nlpp_derivs
Li2_soa_nlpp_noderivs

So I would consider the Li2 test passed and #2083 fixed by this PR. Thank you! Also, PR #2128 or something similar is needed to manage nonlocalpp_derivs with nexus, although it's good to have them now enabled by default.

@jtkrogel
Copy link
Contributor

jtkrogel commented Jan 3, 2020

Thanks @tiihonej. @ye-luo It's nice to have this fix. This is the last update needed prior to AoS removal.

@prckent prckent changed the title [hold] Fix NLPP derivative in SoA Fix NLPP derivative in SoA Jan 3, 2020
@prckent prckent self-requested a review January 3, 2020 19:58
Copy link
Contributor

@prckent prckent left a comment

Choose a reason for hiding this comment

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

LGTM.

@jtkrogel also gave assent over Slack.

@ye-luo ye-luo merged commit c9d071e into QMCPACK:develop Jan 3, 2020
@ye-luo ye-luo deleted the fix-NLPP-deriv-SoA branch January 3, 2020 23:22
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.

NLPP derivatives in optimization need SoA
4 participants