Replies: 7 comments 3 replies
-
|
Yes, I fully agree with the proposed changes. I can see a practical use case for Allowing properties to coexist without a clear specific level of theory introduces ambiguity, which is particularly problematic now that users can export and later reload System objects via JSON. This could easily lead to errors in workflows if a user is not particularly careful. Additionally, maintaining support for |
Beta Was this translation helpful? Give feedback.
-
|
I'm actually an advocate of a If we're okay with things like ONIOM and QM/MM I think we should also be okay with this, obviously warning the user (possibly, very loudly) that they are not performing "standard" operations, but I would not rule out the possibility altogether. As for the burden on devs, I agree we should not be the ones making sure nothing breaks, as this is akin to running the software in an "unintended mode", where all repercussions fall solely on the end user (= carry on at your own risk). Routines and functions should be developed within the strict mode constraints. |
Beta Was this translation helpful? Give feedback.
-
|
Yes @lbabetto, I completely agree with you on the usefulness of a mixed approach, and I’m also in favor of keeping that possibility available to the user. That said, I think there is a subtle but important distinction between the name In fact, with The real difference between
So in short: the current strict mode does allow mixed levels — but only when it’s explicit and non-conflicting across categories. This provides flexibility without losing traceability, which I believe is a good compromise between usability and safety. To clarify what I mean, let me use a deliberately absurd example: from spycci.systems import System
from spycci.engines.xtb import XtbInput
from spycci.engines.orca import OrcaInput
mol = System("water.xyz", 0, 1)
xtb = XtbInput()
orca = OrcaInput(method="PBE", basis_set="def2-SVP")
orca.freq(mol, ncores=4, inplace=True)
print("Before")
print("----------------------------------------------------------")
print(mol.properties.level_of_theory_electronic)
print(mol.properties.level_of_theory_vibronic)
print(mol.properties.vibronic_energy)
if mol.properties.vibrational_data:
print(mol.properties.vibrational_data.frequencies[-1])
else:
print("None")
print()
# Compute a new vibronic property
newmol = xtb.freq(mol, ncores=4)
new_vibronic = newmol.properties.vibronic_energy
mol.properties.set_vibronic_energy(new_vibronic, xtb)
print()
print("After")
print("----------------------------------------------------------")
print(mol.properties.level_of_theory_electronic)
print(mol.properties.level_of_theory_vibronic)
print(mol.properties.vibronic_energy)
if mol.properties.vibrational_data:
print(mol.properties.vibrational_data.frequencies[-1])
else:
print("None")Running with Running with As you can see in both cases different levels of theory are used for vibronic and electronic part. In the second case however the vibrational properties computed by orca are kept with those computed by xtb with an |
Beta Was this translation helpful? Give feedback.
-
|
As a further note: the As an example, building on the one presented before, consider this: Running with Running with This behavior is due to the fact that the xtb engine currently does not implement a parser for vibrational frequencies. The user however has no way of knowing, beside reviewing the code structure, that the frequencies computed by orca and not overwritten by xtb. I think that this should be avoided at all costs. |
Beta Was this translation helpful? Give feedback.
-
|
Yes, perhaps my previous response was a bit superficial, but I also support keeping the mixed mode as described by @lbabetto (in fact, that’s exactly the behavior expected when using What I was referring to, instead, concerns only the Anyway, I believe it's a good idea to inform the user about the selected |
Beta Was this translation helpful? Give feedback.
-
Okay sorry, I misunderstood how this mode is applied then, my bad.
Yes, this is absolutely undesirable behaviour, I don't see any reasonable application for which you'd want to do this. In fact, I would say that mixing different levels of theory for different properties is not running in With this in mind, I agree we should ditch completely the current state of
So, the old |
Beta Was this translation helpful? Give feedback.
-
|
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
Proposal: Simplification and Strengthening of
STRICT_MODEin SPyCCICurrently, SPyCCI provides two policies, selectable through the
spycci.config.STRICT_MODEflag, for handlingSystemproperties originating from calculations performed at different levels of theory.STRICT_MODE=True, the library automatically clears all properties affected by a change in the level of theory, according to the following rules:STRICT_MODE=False, the library allows data from different levels of theory to coexist without any consistency checks and marks the affected level of theory asundefined.In both modes, any change in geometry clears all properties.
I find the
STRICT_MODE=Falsemode of operation both dangerous and not particularly useful. I propose removing it from the library altogether. Storing data under anundefinedlevel of theory is especially risky now that users can save aSystemas a.jsonfile and reload it later for further processing.Proposed Changes
STRICT_MODE=False, keeping only the current behavior ofSTRICT_MODE=True.STRICT: requires equality between the electronic and vibronic levels of theory.VERY_STRICT: requires equality across geometry, electronic, and vibronic levels of theory.If, in the future, we want to support the storage of electronic and vibrational data from multiple levels of theory, I suggest adopting a dictionary-based structure where individually labelled
Propertyobjects associated with different levels of theory can co-exist without ambiguity. This would ensure that all stored data remains clearly identifiable and unambiguous.@lbabetto and @AresValley I would love to hear your thoughts on this.
Beta Was this translation helpful? Give feedback.
All reactions