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

Nexus: add example for specifying estimators #5214

Merged
merged 7 commits into from
Nov 21, 2024

Conversation

aannabe
Copy link
Contributor

@aannabe aannabe commented Nov 8, 2024

Proposed changes

As discussed previously in persion, a comprehensive example was needed for Nexus that demonstrates how to specify various estimators. Here, I have included Density, SpinDenstiy, EnergyDensity, and 1RDM observables in a single Nexus workflow. The goal is to add all the other estimators too as they mature.

In addition, I added a scripts_postproc directory here that includes some post-processing tools. Currently, I included the 1RDM retrieval and plotting tools adapted from discussions in #1052. For now I think it will be helpful to users, at least as a starting point. Let me know if it's not well-suited for here and I will remove it.

For long run, there should be a centralized place for post-processing tools that analyze the estimators in the stat.h5 file. Whether it should be one large tool that is one-fits-all, or multiple specialized smaller tools could also be discussed. Currently, it seems to be not well handled - for instance, I see that qdens breaks if 1RDM or energy density is present in the same stat.h5 file. I will create an issue for this if the fix is not straightforward.

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

  • New example

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

Local machine

Checklist

  • Yes. This PR is up to date with current the current state of 'develop'

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.

Please add the pseudopotential files so that this can eventually be automated and success/failure tracked.

@aannabe
Copy link
Contributor Author

aannabe commented Nov 8, 2024

Pseudos now added.

@aannabe
Copy link
Contributor Author

aannabe commented Nov 11, 2024

Note: I tested that spin-density, 1RDM, and energy-density runs in the batched code with the above Nexus inputs. However, the density estimator crashes with the following error: Fatal Error. Aborting at EstimatorManager input:unparsable <estimator> node, name: density type: density in Estimators input.
Has the density estimator been ported to the batched code? If not, should I keep it or comment it out for now?

@prckent
Copy link
Contributor

prckent commented Nov 11, 2024

I think we decided that since the spindensity estimator was a superset of the density estimator then we didn't need both. However, the error message suggests that is not what is going on here (?). At the very least the error message needs improving. There are other estimators that are close to duplicates of functionality, e.g. the various structure factors.

@aannabe
Copy link
Contributor Author

aannabe commented Nov 11, 2024

Looking at src/Estimators/EstimatorManagerInput.cpp there is no if-clause for the density estimator. It looks like it hasn't been ported. I have removed that estimator from the example to avoid confusion. I have also always used the spin-density estimator to get the total charges.

@jtkrogel
Copy link
Contributor

This is good. Thanks Gani!

If you have time, can you also add the momentum distribution? This is the other one that should have support in the batched drivers already.

@aannabe
Copy link
Contributor Author

aannabe commented Nov 13, 2024

Will add the momentum distribution sometime today.

@aannabe
Copy link
Contributor Author

aannabe commented Nov 13, 2024

Added a minimal input for momentum distribution

Copy link
Contributor

@jtkrogel jtkrogel left a comment

Choose a reason for hiding this comment

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

See comments

@prckent
Copy link
Contributor

prckent commented Nov 14, 2024

I agree on the note about blocks. Best to also add a comment to explain, e.g., "Large numbers (100+) of blocks are preferred for effective statistical analysis. In production runs, more steps per block than specified here are additionally preferred for computational efficiency due to the I/O cost of estimators."

@aannabe
Copy link
Contributor Author

aannabe commented Nov 15, 2024

It looks good to me now. Feel free to push to my branch directly if you think the included comments need refinining.

jtkrogel
jtkrogel previously approved these changes Nov 15, 2024
Copy link
Contributor

@jtkrogel jtkrogel left a comment

Choose a reason for hiding this comment

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

LGTM

#skip_submit = True,
identifier = 'qmc',
path = basepath + 'qmc',
job = job(cores=12,threads=4,app='qmcpack'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Noticed you are using 12 cores here but 16 elsewhere. Any specific reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now changed to 16 as well. I must've copied that section from another example.

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.

I ran this workflow after changing generate_only to 0. The QMC run with the estimators failed. I will poke more later, but did it work for you? If so, which QMCPACK git hash/version?

Fatal Error. Aborting at EstimatorManager input:unparsable <estimator> node, name: edcell type: energydensity in Estimators input.

relevant input section

    <estimators>
         <estimator type="spindensity" name="SpinDensity">
            <parameter name="dr">
               0.05 0.05 0.05
            </parameter>
         </estimator>
         <estimator type="EnergyDensity" name="EDcell" dynamic="e" static="ion0">
            <spacegrid coord="cartesian">
               <origin p1="zero"/>
               <axis p1="a1" scale=".5" label="x" grid="-1 (100) 1"/>
               <axis p1="a2" scale=".5" label="y" grid="-1 (100) 1"/>
               <axis p1="a3" scale=".5" label="z" grid="-1 (100) 1"/>
            </spacegrid>
         </estimator>
         <estimator type="MomentumDistribution" name="nofk" samples="40" kmax="8.0"/>
         <estimator type="OneBodyDensityMatrices" name="DensityMatrices">
            <parameter name="energy_matrix"       >    no                    </parameter>
            <parameter name="integrator"          >    density               </parameter>
            <parameter name="basis"               >
               spo_dm
            </parameter>
            <parameter name="evaluator"           >    matrix                </parameter>
            <parameter name="check_overlap"       >    no                    </parameter>
            <parameter name="samples"             >    10                    </parameter>
         </estimator>
      </estimators>

@aannabe
Copy link
Contributor Author

aannabe commented Nov 21, 2024

EnergyDensity was working for me with the same Nexus estimator definition on another system. Maybe there is something I missed. I will investigate today.

@prckent
Copy link
Contributor

prckent commented Nov 21, 2024

Working now after an update & rebuild.

@aannabe
Copy link
Contributor Author

aannabe commented Nov 21, 2024

Thanks. I also did a short VMC run on this system and it runs with the latest develop. The stat.h5 has all the included estimators:

ElecElec                 Group
IonIon                   Group
Kinetic                  Group
LocalECP                 Group
LocalEnergy              Group
LocalEnergy_sq           Group
LocalPotential           Group
MPC                      Group
NEEnergyDensityEstimator Group
NonLocalECP              Group
OneBodyDensityMatrices   Group
SpinDensity              Group
nofk                     Group

PS. The new DFT+U input style using QE v7.4 also works.

@prckent prckent changed the title Nexus example for specifying estimators Nexus: add example for specifying estimators Nov 21, 2024
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.

Thanks Gani.

@prckent
Copy link
Contributor

prckent commented Nov 21, 2024

Test this please

@ye-luo ye-luo enabled auto-merge November 21, 2024 20:58
@ye-luo ye-luo merged commit 7cb1f18 into QMCPACK:develop Nov 21, 2024
38 of 39 checks passed
@aannabe aannabe deleted the nxs_est_ex branch November 21, 2024 21:36
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