Skip to content

Allow exact spin integration for SOECP alongside wave function optimization#5173

Merged
ye-luo merged 8 commits intoQMCPACK:developfrom
camelto2:fast_soc_evaluateValueAndDerivative
Sep 20, 2024
Merged

Allow exact spin integration for SOECP alongside wave function optimization#5173
ye-luo merged 8 commits intoQMCPACK:developfrom
camelto2:fast_soc_evaluateValueAndDerivative

Conversation

@camelto2
Copy link
Copy Markdown
Contributor

@camelto2 camelto2 commented Sep 19, 2024

Please review the developer documentation
on the wiki of this project that contains help and requirements.

Proposed changez

Describe what this PR changes and why. If it closes an issue, link to it here
with a supported keyword.

This PR enables the use of the exact spin integration in the SOECP alongside wave function optimization. This is both a new feature and a bug fix. Prior to this PR, we set the default execution path of the SOECP to be the exact spin integration, which specifies some of the data structure sizes upon construction. However, there wasn't an evaluateValueAndDerivatives which utilized the exact spin stuff, so there was an array size mismatch since it was going down the code path using the simpsons rule for spin integration and the data structures weren't sized correctly for that. This PR make everything consistent, so that WF optimization will work regardless of which spin integration scheme you use.

Note that I didn't implement the functionality to work with RotatedSPOs for this PR. This just makes sure everything works correctly with normal jastrow optimization. A separate PR for enabling this alongside OrbOpt will happen later when I have time to write a test



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

Delete the items that do not apply

  • Bugfix
  • New feature

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

M1 mac

Checklist

Update the following with a yes where the items apply. If you're unsure about any of them, don't hesitate to ask. This is
simply a reminder of what we are going to look for before merging your code.

  • Yes. This PR is up to date with current the current state of 'develop'
  • Yes. Code added or changed in the PR has been clang-formatted
  • Yes. This PR adds tests to cover any new code, or to catch a bug that is being fixed
  • No. Documentation has been added (if appropriate)

@camelto2 camelto2 changed the title Fast soc evaluate value and derivative Allow exact spin integration for SOECP alongside wave function optimization Sep 19, 2024
Copy link
Copy Markdown
Contributor

@ye-luo ye-luo left a comment

Choose a reason for hiding this comment

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

LGTM. Only need to improve the documentation of functions.

std::vector<ValueType>& ratios,
Matrix<ValueType>& dratios);

// Used by SOECPComponent for faster SOC evaluation
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you document this more like evaluateDerivRatios explaining what is evaluated?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

updated

Vector<ValueType>& dhpsioverpsi)
{
#ifndef QMC_COMPLEX
throw std::runtime_error("SOECPComponent::evaluateValueAndDerivatives should not be called in real build\n");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Need to deal with this in separate PRs. If SOECP only makes sense in complex builds, we should avoid compiling the class completely in real builds and thus we don't need this ifdef.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

anything related to spinors/spin-orbit uses complex wave functions and complex spin functions. So that would be a nice change

Matrix<ValueType>& dratio);

/** Used by SOECPComponent to do faster SOC evaluation with derivatives of ratio
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you document this more like evaluateDerivRatios explaining what is evaluated?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

updated

@ye-luo
Copy link
Copy Markdown
Contributor

ye-luo commented Sep 20, 2024

Test this please

@ye-luo
Copy link
Copy Markdown
Contributor

ye-luo commented Sep 20, 2024

Do we still have any feature relying on the non-exact code path?

@camelto2
Copy link
Copy Markdown
Contributor Author

camelto2 commented Sep 20, 2024

Do we still have any feature relying on the non-exact code path?

In terms of missing functionality, for legacy/single walker multidets right now it would need the slow code path until the evaluateSpinorRatios and evaluateSpinorDeriv is implemented for multidets.

There is no mw_ support in multidets yet for any of the spinor stuff. I just haven't had the time or need for it yet

So single walker multidets is the only piece as far as I'm aware

@ye-luo ye-luo enabled auto-merge September 20, 2024 17:17
@ye-luo ye-luo merged commit da5c057 into QMCPACK:develop Sep 20, 2024
@camelto2 camelto2 deleted the fast_soc_evaluateValueAndDerivative branch September 20, 2024 18:00
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.

2 participants