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

Implement NLPP parameter derivatives in evaluateDerivRatios in JeeI #4177

Merged
merged 2 commits into from
Aug 16, 2022

Conversation

ye-luo
Copy link
Contributor

@ye-luo ye-luo commented Aug 16, 2022

Proposed changes

J3(eeI) NLPP derivatives

What type(s) of changes does this code introduce?

  • New feature
  • Testing changes (e.g. new unit/integration/performance tests)

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

epyc-server

Checklist

  • Yes. This PR is up to date with current the current state of 'develop'

@ye-luo
Copy link
Contributor Author

ye-luo commented Aug 16, 2022

Test this please

@ye-luo
Copy link
Contributor Author

ye-luo commented Aug 16, 2022

@prckent this one is ready for review. Reference numbers are generated in the same way as MSD PR earlier.

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.

Thank Ye. Very good to have these.

Do any of the statistical tests go through this path and will need updating? For the tests we have I would think the change is subtle.

@prckent prckent enabled auto-merge August 16, 2022 19:03
@ye-luo
Copy link
Contributor Author

ye-luo commented Aug 16, 2022

Thank Ye. Very good to have these.

Do any of the statistical tests go through this path and will need updating? For the tests we have I would think the change is subtle.

That is also what I found. The actual change in the total energy is likely small if the NLPP fraction of total energy is already small. However we should not have wrong derivatives, period.

@prckent prckent merged commit b793102 into QMCPACK:develop Aug 16, 2022
@ye-luo ye-luo deleted the JeeI-ratio-deriv branch August 16, 2022 19:57
Copy link
Contributor

@rcclay rcclay left a comment

Choose a reason for hiding this comment

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

Disregard

@ye-luo
Copy link
Contributor Author

ye-luo commented Aug 16, 2022

Disregard

Will add some documentation.

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