Slight inconsistencies in physical constants

[sphuber]

See original issues on aiida-core and aiida-quantumespresso

[greschd]

Should we address this before 2.0? As far as I can see, this comment by @giovannipizzi is the latest suggestion.

Questions on this:

• Is ry_to_ev and ry_si the only inconsistency, or are there others? Probably we should just check all the constants are consistent? Or should we just do whatever QE does, even if it's inconsistent there?
• Do the constants tend to change between QE versions? We could use the QE-versioning code we have now to distinguish these.

I don't think there's much of a point in tracking also the CODATA values - they are defined in scipy.constants.

[giovannipizzi]

I am not sure - I guess we need to check inconsistencies, and double check with QE.
Considering the scope of this library, I would indeed track them with the QE version - we'll probably need to do some git blaming in GitLab to see if/when constants were changed in QE...

[greschd]

Fun!

For the release candidate, at the very least I think we should nail down the interface. How about we have a get_constants(qe_version: Optional[str]) -> Dict[str, float] function for the general case, and then simply

DEFAULT = get_constants()

for the latest ones?

[giovannipizzi]

seems reasonable!

[giovannipizzi]

Anyway, maybe we should just create a default interface, but in the end it might not be needed to create multiple versions?

Here is the blame: https://gitlab.com/QEF/q-e/-/blame/qe-6.5/Modules/constants.f90
and the history: https://gitlab.com/QEF/q-e/-/commits/qe-6.5/Modules/constants.f90
No "real" change (apart for additions of new constants, often derivatives with just a 10^X factor, or, minor irrelevant code changes to that file) happened in the past 12 years, where some constant precision was increased: https://gitlab.com/QEF/q-e/-/commit/a82ffc4c2948ac82c7978898e1ac4b7e99abdf2e

So maybe I wouldn't worry too much about this issue.

The only action I'd take are:

• make sure the constants we use are identical to the ones internal to QE; write this in a comment, saying that we checked it and when, and they haven't changed in the past 12 years
• if not complex, make it easy to have multiple versions in the future. But if it makes everything more complex, let it go...

Then, ensure in aiida-qe we use constants from here for consistency

[greschd]

So.. we just pack the constants into a SimpleNamespace to make sure we can make multiple instances if needed?

What about naming? Should the default be at top-level qe_tools.CONSTANTS? Or qe_tools.constants.DEFAULT? The latter makes more sense if there are different versions, but since there probably won't be I'm not sure which option is better.

[giovannipizzi]

SimpleNamespace

I don't know what this is, I trust your decision here :-)

What about naming? Should the default be at top-level qe_tools.CONSTANTS? Or qe_tools.constants.DEFAULT?

Maybe let's go with qe_tools.CONSTANTS as a pointer to qe_tools.constants.DEFAULT?

[greschd]

SimpleNamespace

I don't know what this is, I trust your decision here :-)

It's basically the same as the AiiDA AttributeDict, except without recursively modifying mappings: see the docs.

What about naming? Should the default be at top-level qe_tools.CONSTANTS? Or qe_tools.constants.DEFAULT?

Maybe let's go with qe_tools.CONSTANTS as a pointer to qe_tools.constants.DEFAULT?

Hm, I think it would make sense to keep the qe_tools.constants.DEFAULT private (e.g., make it qe_tools._constants) for now - so it's fine even if we never need to introduce the versioning.

[ConradJohnston]

While it may not be decided what to do here, can we please document in the code the provenance of the numbers that are used, beyond "they're from AiiDA core"?