MolSSI / QCElemental

Periodic table, physical constants, and molecule parsing for quantum chemistry.
https://molssi.github.io/QCElemental/
BSD 3-Clause "New" or "Revised" License
144 stars 70 forks source link

hbar in constants #198

Open loriab opened 4 years ago

loriab commented 4 years ago

@dgasmith Also, how would you feel about adding all of the atomic units with generic names (e.g. au_distance, au_charge, au_energy) just to make it easy on people so they don't have to remember Bohr, e, Hartree etc.?

That's strange, I commented on this this afternoon, but it's disappeared. I'll try to remember what I wrote:

mattwelborn commented 4 years ago
* The aliases that are there grandfathered from psi4. I don't object to more (and hbar is a good candidate) but I'd like to gather opinions about whether we should be free or sparing with them.

I would prefer to be free. I think it's in the spirit of qcel to have as much chemistry (meta)data as possible.

* Associated is that I aliased the codata 2014 constant names into 2018. how far to we wwant to continue this? 2014 to 2022 or just 2018 to 2022

I think we can keep doing this going forward. We only have to do it once every 4 years, and surely they don't change that many names...

* Note that the constants haven't been systematically fed into pint. there's some it may not recognize:  GeV, T (period)

I propose we feed the constants into pint (with those constants whose units are not supported being silently ignored [or warn at test-time])?

dgasmith commented 4 years ago

I am for more aliases, the only possible downside here is that we contaminate the space too much making it hard to use. I think this may be hard to do (would need ~5-10x the constants we have now).

As a note on items like GeV, pint understands prefixes:

>>> qcel.constants.ureg("GeV")
<Quantity(1, 'gigaelectron_volt')>

We might want to explain the Hz thing in the speed of light as it is a bit awkward in code comments.

dgasmith commented 4 years ago

Moving further comments here:

@loriab:

I suspect that most qcel clients are comfortable with direct physical constants and conversion_factor() seems newfangled. So ureg gets most use as user API and isn't so fully built out. I think we want to keep context and ureg separate as they have different sources.

@dgasmith

To refine, I think we want to keep the PhysicalConstantsContext class, but slowly deprecated the aliases in favor of direct ureg generated ones?

This would be my vote anyways so we have a single source of truth.

loriab commented 4 years ago

To refine, I think we want to keep the PhysicalConstantsContext class, but slowly deprecated the aliases in favor of direct ureg generated ones?

This would be my vote anyways so we have a single source of truth.

I like the idea, but my reservations are: