Skip to content

Conversation

@PDoakORNL
Copy link
Contributor

@PDoakORNL PDoakORNL commented Nov 22, 2025

Proposed changes

Add application level testing for batched vmc and dmc energy.

This brought me up hard against some issues with check_stats.py and the summing tests of the dmc.dat over steps in a block versus the energy density over particles in blocks. I had to figure out just what could be expected here accuracy wise. On the side of the dmc.dat it's less than you would think. I'd still say its not completely settled but I think a reasonable tolerance for these sums is order of the error between the two different ways to get the energy avg of a block from the dmc.dat.

I still think that the energy density sums should match closer to the dmc.dat single average route but they don't by any better a tolerance than the two different ways to get the avg block energy from dmc.dat does. I'm not sure how seriously this should really be taken?

The new estimators follow the rule that we accumulate weighted measurements and we average only once before data is committed. Obviously the output of dmc.dat does not take this route and I added a test in check_stats.py to look at the error this causes, it can be surprising large and so I needed to drop the tolerance of the relative error check for the comparisons to the dmc blocks. I added a new element to the dmc.dat so that the error introduced for block sums can be considered.

this paper
PHYSICAL REVIEW E 83, 066706 (2011)
got me thinking about how we should probably think about the sizes of blocks when collecting observables with DMC a bit more. I haven't run any tests to see how correlated our samples are in the EnergyDensityTests but I get the impression that is an issue.

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

Delete the items that do not apply

  • New feature
  • Testing changes (e.g. new unit/integration/performance tests)
  • Documentation or build script changes
  • Other (please describe):

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

sdgx server

Checklist

Update the following with an [x] 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.

    • I have read the pull request guidance and develop docs
    • This PR is up to date with the current state of 'develop'
    • Code added or changed in the PR has been clang-formatted
    • This PR adds tests to cover any new code, or to catch a bug that is being fixed
    • Documentation has been added (if appropriate)

@PDoakORNL PDoakORNL changed the title [WIP] Enery density app level testing Enery density app level testing Dec 2, 2025
@PDoakORNL PDoakORNL force-pushed the eden_app_level_testing branch from 2c98640 to 31e01a6 Compare December 2, 2025 23:37
@PDoakORNL PDoakORNL marked this pull request as ready for review December 3, 2025 00:05
@PDoakORNL PDoakORNL requested review from jtkrogel and ye-luo December 3, 2025 18:18
Copy link
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.

This is my first impression. I ended with more puzzles. Can we talk over the phone before taking any further actions?

T R2Accepted;
T R2Proposed;
T LivingFraction;
T WeightedEnergySum;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain what is WeightedEnergySum? It sounds like \sum_i w_i LocalE_i over walkers within a population. If that is the case, I feel it is a composite quantity and can be recovered by two existing quantities Weight and Energy because Weight is computed as \sum_i w_i and Energy is computed as (\sum_i w_i LocalE_i)/(\sum_i w_i). For that reason, it seems unnecessary to add and let us keep dmc.dat with minimal output.
If WeightedEnergySum is a different quantity, please elaborate.

Copy link
Contributor

@ye-luo ye-luo Dec 3, 2025

Choose a reason for hiding this comment

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

Second guess. Are you trying to workaround check_stats.py not doing weighted average?

Copy link
Contributor

Choose a reason for hiding this comment

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

Echoing these questions.

else:
sc_vals = sc_vals.mean(1)
#end if
sc_dmc_energies[comparisons[k]] = sc_vals
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you trying to check the block-averaged quantity against the per-step population-averaged values from dmc.dat? If that is the case, we can simply compute Weight * Energy extracted from dmc.dat.

@ye-luo
Copy link
Contributor

ye-luo commented Dec 3, 2025

The new estimators follow the rule that we accumulate weighted measurements and we average only once before data is committed. Obviously the output of dmc.dat does not take this route and I added a test in check_stats.py to look at the error this causes, it can be surprising large.

  1. "weighted measurements" did you mean weight multiplied quantities?
  2. "it can be surprising large", I'm wondering if there is a bug. @jtkrogel Does "check_stats.py" do weighted average or not?

@jtkrogel
Copy link
Contributor

jtkrogel commented Dec 4, 2025

In the past, I was able to get the energy density sum to match (I believe the dmc.dat data) to high precision (given the number of decimal places written to dmc.dat, that is).

If high agreement is there, then the weighting has been done in an identical fashion. Note, this really should be the same between a scalar estimator (the total energy) and a "collectable" one (the energy density) at each step.

Sum rules should be easy to enforce...

@jtkrogel
Copy link
Contributor

jtkrogel commented Dec 4, 2025

If you want to check more closely, just run with 1 step per block. Then each energy entry in scalar.dat should match each entry in dmc.dat. The volume integral of each energy density entry in stat.h5 should match each of the scalar.dat and dmc.dat values.

@jtkrogel
Copy link
Contributor

jtkrogel commented Dec 4, 2025

Also, the definition of the weighting should be unambiguous. See e.g. equation 3.53 in Foulkes 2001 and equation 24.21 in David's book "Interacting Electrons".

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.

4 participants