Misleading definition of GPa conversion factor

max-veit commented 7 years ago

In Units.f95 (and quippy.units.GPA), the gigapascal conversion unit is defined as the inverse of what it should be - e.g. to convert from quip units (eV Å^-3) to GPa, you would expect to do

press / quippy.units.GPA # WRONG!

but it's the other way around (this converts from GPa to quip units)

Since so much code uses the existing constant*, we can't just change it. In order to avoid tripping up new users, it's been proposed to introduce a new constant, EV_A3_IN_GPA with the same value. This makes the correct use in conversions much clearer. The existing GPA constant could then be slowly deprecated and a new constant introduced for the more intuitive (well, more consistent) conversion factor, i.e. the inverse of the one currently defined.

*(several uses in each of the following files):

src/libAtoms/Barostat.f95
src/libAtoms/DynamicalSystem.f95
src/libAtoms/Units.f95
src/Potentials/Potential.f95
src/Potentials/Potential_Hybrid_utils.f95
src/Potentials/Potential_Precon_Minim.f95
src/Programs/md.f95
src/Programs/md_gid.f95
src/Programs/quip.f95
src/Programs/slice_sample.f95
src/Utils/cracktools.f95
src/Utils/elasticity.f95
examples/bulk-mod.py
examples/elastic.py
examples/quartz-strain.py
quippy/asap.py
quippy/castep.py
quippy/crack.py
quippy/molpro.py
scripts/bulk_properties.py

max-veit commented 7 years ago

Related is this issue -- basically, the comment in the Fortran file that defines this constant explains its correct usage, but that comment doesn't make it to the online documentation.

gabor1 commented 7 years ago

sounds like a sensible course of action.

alternatively, what about changing the value of the constant, and search-replacing the units.GPA string and replacing it with (1/units.GPA) ?

-- Gábor

On 26 July 2017 at 16:40:48, Max Veit (notifications@github.com) wrote:

Related is this issue
-- basically, the comment in the Fortran file that defines this constant explains its
correct usage, but that comment doesn't make it to the online documentation.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/libAtoms/QUIP/issues/82#issuecomment-318092919

max-veit commented 7 years ago

That would cause every other user who used GPA somewhere in their code to suddenly start getting wrong results.

On 26 Jul 2017 6:39 p.m., "gabor1" notifications@github.com wrote:

sounds like a sensible course of action.

alternatively, what about changing the value of the constant, and search-replacing the units.GPA string and replacing it with (1/units.GPA) ?

-- Gábor

On 26 July 2017 at 16:40:48, Max Veit (notifications@github.com) wrote:

Related is this issue -- basically, the comment in the Fortran file that defines this constant explains its correct usage, but that comment doesn't make it to the online documentation.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/libAtoms/QUIP/issues/82#issuecomment-318092919

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/libAtoms/QUIP/issues/82#issuecomment-318127940, or mute the thread https://github.com/notifications/unsubscribe-auth/AAr7MjpaXtRLbidpUlQPV1gxBzZKEOV1ks5sR3m6gaJpZM4OkGmN .

gabor1 commented 7 years ago

you mean in code that wasn’t in the quip repo. true. let’s create a new constant then.

-- Gábor

On 26 July 2017 at 19:10:04, Max Veit (notifications@github.com) wrote:

That would cause every other user who used GPA somewhere in their code to suddenly start getting wrong results.

On 26 Jul 2017 6:39 p.m., "gabor1" wrote:

sounds like a sensible course of action.

alternatively, what about changing the value of the constant, and search-replacing the units.GPA string and replacing it with (1/units.GPA) ?

-- Gábor

On 26 July 2017 at 16:40:48, Max Veit (notifications@github.com) wrote:

Related is this issue -- basically, the comment in the Fortran file that defines this constant explains its correct usage, but that comment doesn't make it to the online documentation.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/libAtoms/QUIP/issues/82#issuecomment-318092919

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub , or
mute the thread

.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/libAtoms/QUIP/issues/82#issuecomment-318136720

libAtoms / QUIP

Misleading definition of GPa conversion factor #82