Open hheenen opened 3 years ago
Thanks. it's likely a consequence of a recent change to support the stress
key at all, previously it was just virial
. @bernstei what are the precedence rules if both keys are present?
Virial should override stress, but stress is used if virial is missing. Not well documented, but it is in gap_fit --help
:
stress_parameter_name type=STRING scalar current_value=stress
Name of stress property (6-vector or 9-vector) in the input XYZ file that descri-
bes the data - stress values only used if virials are not available (opposite
sign, standard Voigt order)
Is this a problem? I want to deprecate virial, really.
the reason I like the virial is because the virial sigma is in those units.
-- Gábor
Gábor Csányi Professor of Molecular Modelling Engineering Laboratory, University of Cambridge Pembroke College Cambridge
Pembroke College supports CARA. A Lifeline to Academics at Risk. http://www.cara.ngo/
On 26 Oct 2021, at 15:04, bernstei @.***> wrote:
Virial should override stress, but stress is used if virial is missing. Not well documented, but it is in
gap_fit --help
: stress_parameter_name type=STRING scalar current_value=stress Name of stress property (6-vector or 9-vector) in the input XYZ file that descri- bes the data - stress values only used if virials are not available (opposite sign, standard Voigt order)Is this a problem? I want to deprecate virial, really. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.
On Oct 26, 2021, at 10:11 AM, gabor1 @.***> wrote:
the reason I like the virial is because the virial sigma is in those units.
It's a big hassle with ASE, though, and that's the primary way we create configuration files these days.
yes I am conscious of that. So from ASE, I guess the stress is easy to get out, and if you don’t do anything, there is no virial at all, and presumably most people use it in this way?
-- Gábor
Gábor Csányi Professor of Molecular Modelling Engineering Laboratory, University of Cambridge Pembroke College Cambridge
Pembroke College supports CARA. A Lifeline to Academics at Risk. http://www.cara.ngo/
On 26 Oct 2021, at 15:23, bernstei @.***> wrote:
On Oct 26, 2021, at 10:11 AM, gabor1 @.***> wrote:
the reason I like the virial is because the virial sigma is in those units.
It's a big hassle with ASE, though, and that's the primary way we create configuration files these days. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.
On Oct 26, 2021, at 10:25 AM, gabor1 @.***> wrote:
yes I am conscious of that. So from ASE, I guess the stress is easy to get out, and if you don’t do anything, there is no virial at all, and presumably most people use it in this way?
It's definitely what I do. And it's consistent with the new approach that we think of libAtoms as a vestigial thing, to be replaced by ASE when possible. Certainly quippy.potential.Potential needs to support stress rather than virial, because that's a valid key for calc.results. The file format does not have to be consistent with that, but it's a bit simpler.
For what it's worth, I don't like stress automatically replacing virial (even though I strongly support the stress being supported). When we are prototyping a GAP, we often do things like "viriral_parameter_name=DUMMY" (and the same with forces) to fit a quick and dirty potential with only energies, perhaps on a local machine. This used to work, even if "stress" is in the XYZ file, and ASE puts it in there by default when, e.g., importing a VASP OUTCAR. But when we do this now, the virials are read in from the listed stress in the XYZ file, and we end up (unadvertedly) fitting a model that requires a lot more memory/time.
I think we need to pick one of stress and virial for extxyz, and stick to it. Everything else will lead to confusion of various sorts (not actually fitting to virial because you forgot you only provided stress, fitting to the wrong one, etc). But this applies to both gap_fit
and for the extxyz format, I'd argue. And if the latter is marketed as a "normal" ASE format, it really makes for sense for it to be stress
. I suppose we could give gap_fit
an explicit use_virial/use_stress
keyword, and then it'll at least not be ambiguous, but I still think we should suck it up and pick one (and I vote for stress, all in all).
On Oct 26, 2021, at 11:30 AM, Miguel A. Caro @.**@.>> wrote:
For what it's worth, I don't like stress automatically replacing virial (even though I strongly support the stress being supported). When we are prototyping a GAP, we often do things like "viriral_parameter_name=DUMMY" (and the same with forces) to fit a quick and dirty potential with only energies, perhaps on a local machine. This used to work, even if "stress" is in the XYZ file, and ASE puts it in there by default when, e.g., importing a VASP OUTCAR. But when we do this now, the virials are read in from the listed stress in the XYZ file, and we end up (unadvertedly) fitting a model that requires a lot more memory/time.
BTW, is there a reason you can't just do "`virial_parameter_name=DUMMY stress_parameter_name=DUMMY"?
Yes I can do this, I was just illustrating the "confusion" you were referring to in your previous post. I agree to stick to one and also that stress should be preferred. And a choice between using virial and stress (with a stress default) would be nice. There is also the question of how to handle regularization that Gabor talked about.
In the newer versions of the /src/GAP, the behavior of the “gap_fit” command changed. Independent of how the
virial_parameter_name
is specified (or at all for that matter), the virials appear to be read-out if a “stress” entry in the extxyz file is present. Since this is automatically added byase.io.write_xyz
if the stress is found in an atoms object, this may occur frequently. The older /src/GAP version ignores the virials if novirial_parameter_name
is specified.I am not sure if this behavior is intended. Naturally, its only a small change and there are many easy work-arounds if it poses a problem. I just wanted to mention it here, if not aware, one may get confused by this.
PS: I can't tell when this behavior in /src/GAP changed since the GAP distribution was only included recently in the QUIP repo. It did not yet occur in a downloaded tarball from October 2020.
Many thanks!