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

Orbital rotation optimization test for Be atom #4619

Merged
merged 3 commits into from
Jun 2, 2023

Conversation

markdewing
Copy link
Contributor

Test Be atom with STO basis. The coefficient matrix is partially optimized, taken from an optimizer run that started from the identity coefficient matrix. This tests the remainder of the optimization.

The test takes about a minute on a Broadwell CPU.

The input file does 3 iterations with minwalkers=0.1, and 3 iterations with minwalkers=0.95 (for reference, default minwalkers is 0.3). After new parameters are calculated, a correlated sampling run is done and the effective number of walkers is computed. If it is less than minwalkers, the new parameters are rejected, and the next optimizer loop iterations proceeds.

A run using the default parameters produces large variations in later optimizer iterations.
be_opt_default

It could be that this is such a small system that parameter moves with a large overlap can still produce a poor energy. Using stricter minwalkers cutoff in the last 3 iterations seems to alleviate the issue.

be_opt_minwalkers 95

It could also relate to the intrinsic noise. Adding more samples helps (at the cost of a longer test). A run using these orbitals with an e-e Jastrow factor had an order of magnitude less variance in the energy, and did not show optimizer histories with poor energies.

In both graphs, the last iteration is a VMC run.

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

Delete the items that do not apply

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

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

desktop

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'
  • N/A. 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
  • N/A. Documentation has been added (if appropriate)

Test Be atom with STO basis.  The coefficient matrix is partially
optimized, taken from an optimizer run that started from the identity
coefficient matrix.  This tests the remainder of the optimization.
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.

What reliability do you expect with the 0.03 errorbar and the current test inputs?

From the graph it looks like we would need a higher value for this test to nearly always work (>>95%).

Otherwise LGTM. Good the Jastrow version is even better.

@ye-luo
Copy link
Contributor

ye-luo commented Jun 2, 2023

I assume the optimal miniwalkers value is correlated with the error bar of the statistics. When the number of samples is small or the WF is in poor quality, outlier samples cause more optimization stability issues and thus requires a miniwalkers value close to 1.

@ye-luo
Copy link
Contributor

ye-luo commented Jun 2, 2023

The second graph shows there is still failure with miniwalkers 0.95. If we add a fixed Jastrow from to the fully optimized WF, does the optimization test become more robust? If so, I would like to have tests with the fixed Jastrow added.

@markdewing
Copy link
Contributor Author

Two possibilities:

  1. Running 4 steps with minwalkers at the default seems to be stable, with sufficiently large error bounds.
  2. Adding the Jastrow factor is much more stable and repeatable, but the tradeoff is there is no externally validated answer.

And default minwalkers.  Seems stable.
@markdewing
Copy link
Contributor Author

markdewing commented Jun 2, 2023

I updated the test for the first alternative. Since there are fewer optimizer iterations, it runs a little faster as well.
be_opt_4_iterations

@prckent
Copy link
Contributor

prckent commented Jun 2, 2023

Adding the Jastrow factor is much more stable and repeatable, but the tradeoff is there is no externally validated answer.

Still worth adding in a later PR imo. All of our vmc tests with jastrows and all our dmc tests have this issue, but it does help us catch new breakage.

@prckent
Copy link
Contributor

prckent commented Jun 2, 2023

Just a comment, but isn't it interesting that we see only ~two sets of energies on the first optimization step and not a continuum of possible results? Why would that be?

(Assuming the updated test gets through CI, I suggest we merge it and then keep an eye on the pass/fail rate and adjust the acceptable error bar accordingly)

@prckent
Copy link
Contributor

prckent commented Jun 2, 2023

Test this please

@markdewing
Copy link
Contributor Author

I think the discrete splits are due to rejected parameter moves.

@prckent
Copy link
Contributor

prckent commented Jun 2, 2023

So the higher split set of energies after one iteration actually uses the same parameters as the zeroth iteration?

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

@prckent
Copy link
Contributor

prckent commented Jun 2, 2023

Test this please

@prckent prckent enabled auto-merge June 2, 2023 17:14
@markdewing
Copy link
Contributor Author

Yes, the same variational parameters get used. I would expect it would have the same distribution as in the first iteration. There's only 4 runs that had the first move rejected, so it's a very small sample size.
One of the algorithm parameters is adjusted when a parameter move is rejected (shift_s, which mixes the overlap matrix into the Hamiltonian matrix). That adjustment is why the parameter move (and energy) is different between the rejected moves and accepted moves (ie. the jump from 0->1 for accepted moves is different from 1->2 for rejected moves)

@prckent prckent merged commit 673e238 into QMCPACK:develop Jun 2, 2023
36 of 37 checks passed
@markdewing markdewing deleted the be_sto_opt branch June 2, 2023 19:15
@prckent prckent mentioned this pull request Aug 18, 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.

None yet

3 participants