QMCPACK / qmcpack

Main repository for QMCPACK, an open-source production level many-body ab initio Quantum Monte Carlo code for computing the electronic structure of atoms, molecules, and solids with full performance portable GPU support
http://www.qmcpack.org
Other
300 stars 139 forks source link

Guard against edens and 1rdm in qdens #5221

Closed aannabe closed 1 week ago

aannabe commented 1 week ago

Proposed changes

If one accumulates SpinDensity estimator coupled with either OneBodyDensityMatrices or NEEnergyDensityEstimator, qdens breaks with:

Traceback (most recent call last):
  File "/home/wavefunction/codes/qmcpack/nexus/bin/qdens", line 1452, in <module>
    qdens.process()
  File "/home/wavefunction/codes/qmcpack/nexus/bin/qdens", line 1428, in process
    stat.analyze(eq,rb)
  File "/home/wavefunction/codes/qmcpack/nexus/bin/qdens", line 756, in analyze
    q_data = reblock(density.value,equilibration,reblock_factor,self.filepath)
                     ^^^^^^^^^^^^^
AttributeError: 'HDFgroup' object has no attribute 'value'. Did you mean: 'values'?

This is because qdens looks for the word "density" to identify the usual density estimator and gets tricked by other observable names. A straightforward fix for this is to hardcode the estimator names that qdens is looking for as spindensity and density. Note that the docs specifies that they must be spindensity and density anyway:

https://qmcpack.readthedocs.io/en/develop/hamiltonianobservable.html?highlight=observables#density-estimator https://qmcpack.readthedocs.io/en/develop/hamiltonianobservable.html?highlight=observables#spin-density-estimator

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

Does this introduce a breaking change?

What systems has this change been tested on?

Local machine

Checklist

jtkrogel commented 1 week ago

Can't estimators have any name at all? Generally it is set by the user.

If instead the name becomes restricted to a single value, this prevents multiple estimators of the same type (a pattern generally allowed e.g. w/ multiple Jastrows, etc.).

ye-luo commented 1 week ago

Can't estimators have any name at all? Generally it is set by the user.

If instead the name becomes restricted to a single value, this prevents multiple estimators of the same type (a pattern generally allowed e.g. w/ multiple Jastrows, etc.).

To support more than one estimator of the same type, both name and typename must be recorded for each dataset.

aannabe commented 1 week ago

@jtkrogel Any suggestions for circumventing the described issue without constraining density names? I agree with @ye-luo that the cleanest would be if the stat.h5 had contained the type in addition to the name. But currently, I am quite confused by what goes to stat.h5 file - there doesn't seem to be a set standard. For example, the spindensity estimator respects the user input for name tag, and that is what I see in the stat.h5 file. But the EnergyDensity and OneBodyDensityMatrices do not seem to respect user's name input and just put either something unexpected or the type name.

Being able to run multiple instances of the same estimator would be great. Recently, I tested that in the OneBodyDensityMatrices by specifying two distinct 1RDMs - one with spin-up basis and the other with spin-down basis in the same input file. But I found that there is only one 1RDM in the stat.h5.

jtkrogel commented 1 week ago

Perhaps it's good enough to filter out the common ones? e.g.

if 'density' in lname and 'energy' not in lname and 'onebody' not in lname:
    ...

The general solution is for QMCPACK to record the value of type in the HDF group containing the data for each estimator. This way the data is self-documenting and one is not relying on the user to embed specific patterns in the name.

jtkrogel commented 1 week ago

EnergyDensity and OneBodyDensityMatrices need to be fixed to at least be consistent with current usage across estimator.

aannabe commented 1 week ago

Should be good to go now I think.

ye-luo commented 1 week ago

Test this please