Skip to content

Self-Healing Overlap Estimator#4991

Merged
ye-luo merged 29 commits intoQMCPACK:developfrom
jtkrogel:sh_overlap_pr
May 17, 2024
Merged

Self-Healing Overlap Estimator#4991
ye-luo merged 29 commits intoQMCPACK:developfrom
jtkrogel:sh_overlap_pr

Conversation

@jtkrogel
Copy link
Copy Markdown
Contributor

Proposed changes

Add implementation of self-healing overlap estimator for legacy and batched drivers.

As requested, a 4 mpi 4 omp-thread deterministic test is included. The check_stats.py testing script was updated to handle deterministic tests for stat.h5 quantities based on an absolute tolerance check in addition to the statistical check capability already present.

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?

Laptop

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. This PR adds tests to cover any new code, or to catch a bug that is being fixed

@jtkrogel jtkrogel requested a review from ye-luo May 14, 2024 20:28
/** Compute the derivatives of the log of the wavefunction component with respect to the optimizable parameters.
* Unlike evaluateDerivativesWF, knowledge of mappings from component parameters to the full set of wavefunction parameters is not required.
*/
virtual void evaluateDerivativesWF_local(Vector<ValueType>& dlogpsi);
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.

Please remove from WFC and keep it only to MSD.

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.

This function is unused, removed now.

Matrix<ValueType>& dratios);


virtual void detRatios(Vector<ValueType>& ratios)
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.

Please remove from WFC and keep it only to MSD.

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.

What does this function calculate?

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.

Removed from WFC, functionality only at MSD level now.

Inline documentation added.

<parameter name="subSteps" > 2 </parameter>
<parameter name="timestep" > 0.4 </parameter>
<parameter name="warmupSteps" > 2 </parameter>
<estimator type="selfhealingoverlap" name="sh_overlap" />
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.

@PDoakORNL is bare estimator still allowed?

Copy link
Copy Markdown
Contributor Author

@jtkrogel jtkrogel May 15, 2024

Choose a reason for hiding this comment

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

The batched input file for this test has now been updated to use the block similar to the batched spin-density test for diamond.

@jtkrogel
Copy link
Copy Markdown
Contributor Author

@ye-luo I've addressed all of your comments/requests.

@jtkrogel
Copy link
Copy Markdown
Contributor Author

Not expecting to add more commits. Ready for merge.

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.

Overall OK. I will do some refactoring later to avoid the redundant code added by detRatios.

@ye-luo
Copy link
Copy Markdown
Contributor

ye-luo commented May 15, 2024

Test this please

@ye-luo ye-luo enabled auto-merge May 15, 2024 14:27
@ye-luo ye-luo disabled auto-merge May 15, 2024 15:32
@ye-luo
Copy link
Copy Markdown
Contributor

ye-luo commented May 15, 2024

The CI failure seems mysterious. The failure is clouded by #4997

default_label = 'sh_coeff',
data_paths = obj(coeff='value'),
),
})
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.

Incorrect mapping in the above caused test failure. Please check against the old table.

        quantity_paths = obj({
        qpaths = stat_info[options.quantity].data_paths
            'density'       : obj(tot='value'),
            'spindensity'   : obj(u='u/value',
                                  d='d/value'),
            '1rdm'          : obj(u='number_matrix/u/value',
                                  d='number_matrix/d/value'),
            '1redm'         : obj(u='energy_matrix/u/value',
                                  d='energy_matrix/d/value'),
            'obdm'          : obj(u='number_matrix/u/value',
                                  d='number_matrix/d/value'),
            'energydensity' : obj(W=('spacegrid1/value',0,3),
                                  T=('spacegrid1/value',1,3),
                                  V=('spacegrid1/value',2,3)),
            'momentum'      : obj(tot='value'),

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.

Fixing test failure required.

@jtkrogel
Copy link
Copy Markdown
Contributor Author

Fixed. Estimator tests are now operable:

>ctest -R deterministic-LiH_ae_msdj
Test project /home/j1k/repositories/qmcpack_dev_new/build_real
    Start 611: deterministic-LiH_ae_msdj_legacy-vmc-estimator-sh_coeff-4-4
1/4 Test #611: deterministic-LiH_ae_msdj_legacy-vmc-estimator-sh_coeff-4-4 ..........   Passed    9.29 sec
    Start 612: deterministic-LiH_ae_msdj_legacy-vmc-estimator-sh_coeff-4-4-check
2/4 Test #612: deterministic-LiH_ae_msdj_legacy-vmc-estimator-sh_coeff-4-4-check ....   Passed    0.36 sec
    Start 613: deterministic-LiH_ae_msdj_batched-vmc-estimator-sh_coeff-4-4
3/4 Test #613: deterministic-LiH_ae_msdj_batched-vmc-estimator-sh_coeff-4-4 .........   Passed    7.96 sec
    Start 614: deterministic-LiH_ae_msdj_batched-vmc-estimator-sh_coeff-4-4-check
4/4 Test #614: deterministic-LiH_ae_msdj_batched-vmc-estimator-sh_coeff-4-4-check ...   Passed    0.36 sec

>ctest -R deterministic-diamondC_1x1x1_pp-dmc-estimator
Test project /home/j1k/repositories/qmcpack_dev_new/build_real
      Start 1125: deterministic-diamondC_1x1x1_pp-dmc-estimator-density-1-1
 1/16 Test #1125: deterministic-diamondC_1x1x1_pp-dmc-estimator-density-1-1 .............   Passed    1.21 sec
      Start 1126: deterministic-diamondC_1x1x1_pp-dmc-estimator-density-1-1-check
 2/16 Test #1126: deterministic-diamondC_1x1x1_pp-dmc-estimator-density-1-1-check .......   Passed    0.35 sec
      Start 1127: deterministic-diamondC_1x1x1_pp-dmc-estimator-density-4-4
 3/16 Test #1127: deterministic-diamondC_1x1x1_pp-dmc-estimator-density-4-4 .............   Passed   11.62 sec
      Start 1128: deterministic-diamondC_1x1x1_pp-dmc-estimator-density-4-4-check
 4/16 Test #1128: deterministic-diamondC_1x1x1_pp-dmc-estimator-density-4-4-check .......   Passed    0.36 sec
      Start 1135: deterministic-diamondC_1x1x1_pp-dmc-estimator-spindensity-1-1
 5/16 Test #1135: deterministic-diamondC_1x1x1_pp-dmc-estimator-spindensity-1-1 .........   Passed    1.01 sec
      Start 1136: deterministic-diamondC_1x1x1_pp-dmc-estimator-spindensity-1-1-check
 6/16 Test #1136: deterministic-diamondC_1x1x1_pp-dmc-estimator-spindensity-1-1-check ...   Passed    0.36 sec
      Start 1137: deterministic-diamondC_1x1x1_pp-dmc-estimator-spindensity-4-4
 7/16 Test #1137: deterministic-diamondC_1x1x1_pp-dmc-estimator-spindensity-4-4 .........   Passed   11.43 sec
      Start 1138: deterministic-diamondC_1x1x1_pp-dmc-estimator-spindensity-4-4-check
 8/16 Test #1138: deterministic-diamondC_1x1x1_pp-dmc-estimator-spindensity-4-4-check ...   Passed    0.36 sec
      Start 1145: deterministic-diamondC_1x1x1_pp-dmc-estimator-1rdm-1-1
 9/16 Test #1145: deterministic-diamondC_1x1x1_pp-dmc-estimator-1rdm-1-1 ................   Passed    1.85 sec
      Start 1146: deterministic-diamondC_1x1x1_pp-dmc-estimator-1rdm-1-1-check
10/16 Test #1146: deterministic-diamondC_1x1x1_pp-dmc-estimator-1rdm-1-1-check ..........   Passed    0.36 sec
      Start 1147: deterministic-diamondC_1x1x1_pp-dmc-estimator-1rdm-4-4
11/16 Test #1147: deterministic-diamondC_1x1x1_pp-dmc-estimator-1rdm-4-4 ................   Passed   16.40 sec
      Start 1148: deterministic-diamondC_1x1x1_pp-dmc-estimator-1rdm-4-4-check
12/16 Test #1148: deterministic-diamondC_1x1x1_pp-dmc-estimator-1rdm-4-4-check ..........   Passed    0.37 sec
      Start 1151: deterministic-diamondC_1x1x1_pp-dmc-estimator-momentum-1-1
13/16 Test #1151: deterministic-diamondC_1x1x1_pp-dmc-estimator-momentum-1-1 ............   Passed    1.09 sec
      Start 1152: deterministic-diamondC_1x1x1_pp-dmc-estimator-momentum-1-1-check
14/16 Test #1152: deterministic-diamondC_1x1x1_pp-dmc-estimator-momentum-1-1-check ......   Passed    0.37 sec
      Start 1153: deterministic-diamondC_1x1x1_pp-dmc-estimator-momentum-4-4
15/16 Test #1153: deterministic-diamondC_1x1x1_pp-dmc-estimator-momentum-4-4 ............   Passed   11.62 sec
      Start 1154: deterministic-diamondC_1x1x1_pp-dmc-estimator-momentum-4-4-check
16/16 Test #1154: deterministic-diamondC_1x1x1_pp-dmc-estimator-momentum-4-4-check ......   Passed    0.36 sec

ye-luo
ye-luo previously approved these changes May 16, 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.

I left two requests to change mostly for myself to do some changes in a later PR #5000 but feel free to comment.

* These are obtained via derivatives of the log of the wavefunction component with respect to the optimizable parameters.
* Unlike evaluateDerivativesWF, knowledge of mappings from component parameters to the full set of wavefunction parameters is not required.
*/
void detRatios(Vector<ValueType>& ratios);
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.

This function doesn't conform naming rules that functions start with a verb in lower case. I would recommend calcIndividualDetRatios.
Although calculating individual determinant ratios and calculating parameter derivatives share very similar internal implementation in the case of CI not necessarily the case with CSFs. They are conceptually different from APIs perspective. The difference is larger than just a mapping for example the det 0 is skipped when calculating derivatives. So I would recommend removing

 These are obtained via derivatives of the log of the wavefunction component with respect to the optimizable parameters.
   *  Unlike evaluateDerivativesWF, knowledge of mappings from component parameters to the full set of wavefunction parameters is not required.`

in the documentation. Please also document the argument ratios.

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.

Also please test this function at

https://github.com/QMCPACK/qmcpack/blob/2764f973212f911490ff488a44ed011b22678154/src/QMCWaveFunctions/tests/test_multi_slater_determinant.cpp#L112

although you'll need to findMSD and cast the type to MSD first.
Please test the first element and a non-first element.

@ye-luo
Copy link
Copy Markdown
Contributor

ye-luo commented May 16, 2024

Test this please

@ye-luo ye-luo enabled auto-merge May 16, 2024 19:07
@ye-luo ye-luo mentioned this pull request May 16, 2024
@ye-luo
Copy link
Copy Markdown
Contributor

ye-luo commented May 16, 2024

Hit one test failure https://github.com/QMCPACK/qmcpack/actions/runs/9117814869/job/25069532490
It is mixed precision. Probably just increase tolerance?

@jtkrogel
Copy link
Copy Markdown
Contributor Author

Fixed the mixed precision tests. Should be good to merge now.

@ye-luo
Copy link
Copy Markdown
Contributor

ye-luo commented May 17, 2024

Test this please

@ye-luo ye-luo merged commit a224ca6 into QMCPACK:develop May 17, 2024
@jtkrogel jtkrogel deleted the sh_overlap_pr branch November 19, 2024 22:37
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