Skip to content

More application testing energy density estimator#6011

Open
PDoakORNL wants to merge 4 commits into
developfrom
eden_app_level_synthetic_pr
Open

More application testing energy density estimator#6011
PDoakORNL wants to merge 4 commits into
developfrom
eden_app_level_synthetic_pr

Conversation

@PDoakORNL

@PDoakORNL PDoakORNL commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Proposed changes

Additional Testing of Energy Density Estimator

Added:
Homogenous Electron Gas tests for batched vmc and dmc.
The energy density should be approximately homogenous.
The energy should be the same whether the coulomb AA and other hamiltonian operators should produce the same results whether batched per particle eval is taken or not. It is taken when the energy density estimator is on

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

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

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

osx dev laptop

Checklist

    • 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

Copy link
Copy Markdown
Contributor Author

These failing checks are all failures on the CI side. I think #6016 may address one but something with the coverage is also broken.

@PDoakORNL
PDoakORNL marked this pull request as ready for review July 17, 2026 15:01

@prckent prckent left a comment

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.

LGTM

Good to have short batched tests of these where we previously had none.

@ye-luo ye-luo left a comment

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.

Only a few clarification questions on check_stats.py.

sc_vals = scalars.data[comparisons[k]]
if scalars.file_type=='dmc':
if len(sc_vals)%len(ed_vals)==0 and len(sc_vals)>len(ed_vals):
print("len(sc_vals)%len(ed_vals)", len(sc_vals)%len(ed_vals), len(sc_vals), " >= ", len(ed_vals))

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.

Is this line for debugging or intended to be visible to users? See quite casual printout.

sc_vals = sc_vals.sum(1)
sc_weights = sc_weights.reshape(len(sc_weights)//steps,steps)
sc_weights = sc_weights.sum(1)
sc_vals = sc_vals / sc_weights

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.

It seems to be an important bug fix getting proper weighted average now. Did I understand it correct? I didn't find anything mentioned in the PR description. So wondering.

# This test is actually relative error assuming the
# the scv is the true value. since conventionally the
# divisor is the true value.
print("checking estimator", comparisons[k], "versus scalar agreement")

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.

Is this just for debugging?

if scalars.file_type=='dmc':
ftol = 1e-7
else:
ftol = 1e-7

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.

Same value for if and else?

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