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

[WIP] Next orbital rotation prototype #4351

Closed
wants to merge 21 commits into from

Conversation

markdewing
Copy link
Contributor

@markdewing markdewing commented Dec 1, 2022

Supersedes #4202

Changes from that version:

  • For splines, the coefficients can be saved in order to enable the global rotation method. The multiplication was changed slightly to copy the old spline coefficients to the temporary storage first, then store the results of the multiplication in the actual spline storage.
  • The coefficient copy uses a shared pointer to reduce storage. The LCAO implementation was changed. This reduces the extra storage needed. Only one thread should apply the rotation
  • Hack to specify the rotation method in the input file. Default is history list. Use 'optimize="global"' to use global rotation method.

Updates:

  • Works for the single-determinant case. Implemented evaluteDerivativesWF in RotatedSPOs and a path to calling it.
  • Works with batched algorithm for NLPP in the single-determinant case. Implemented evaluateDerivRatios in RotatedSPOs and a path to calling it, along with extra arguments.

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

Delete the items that do not apply

  • New feature

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

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
  • No. 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)

Now the stored coefficient are a shared pointer, only perform
the rotation on a single thread.
Hack to specify the rotation method in the input file.
Default is history list.  Use 'optimize="global"' to use global
rotation.
Otherwise an assert triggers with global rotation and multi-det.
@jptowns
Copy link
Contributor

jptowns commented Dec 7, 2022

This may not be the right place for this, but want to get out in the open so that we remember to circle back at some point. During optimization (oneshift at least but possibly for other methods), a rotation with a proposed set of wfn parameters is applied before deciding to accept/reject the proposed parameters. This is done, e.g. in line 1365 in QMCFixedSampleLinearOptimize.cpp in this line: const RealType newCost = optTarget->Cost(false);. If the parameters are accepted, it's fine. But if th eparameters are rejected, we need to apply the old rotation, otherwise we're stuck with the (now rejected) parameters. I thikn that the simplest possible fix would be to simply add another optTarget->Cost() call after rejection. But we should probably study the optimizer(s) more closely to see where else this may crop up.

Otherwise, this PR looks good to me.

This is a copy and paste to get *something* working for the single
determinant case.
In order to compute the delta rotation properly, myVars needs to
stay in sync with the values in the driver.
It always must be set to the value passed in to resetParametersExclusive.

Also set initial values to match testing.
At least compute the det ratios if evaluateDerivRatios is not
implemented.  This matches the implementation in WaveFunctionComponent.

This fixes the deterministic test failure in
diamondC_1x1x1_pp-param-grad.
Accumulate the parameter derivatives correctly in the case
where multiple wavefunction components use the same parameter.
@prckent
Copy link
Contributor

prckent commented Feb 15, 2023

Q. How much of this remains to be cleaved off and merged? Can we close this now? What is needed to run (say) FeO in splines at gamma in the batched code?

@markdewing
Copy link
Contributor Author

The key changes to enable orbital rotation are the functions checkInVariablesExclusive and resetParametersExclusive in RotatedSPOs.
What remains

What's left after that is the saving and restoring of the rotation parameters

@markdewing markdewing closed this Feb 15, 2023
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