Some cli renames #241
Some cli renames #241
Conversation
wfl/cli/commands/calc.py
Outdated
| @@ -104,3 +106,22 @@ def atomization_energy(inputs, outputs, prop_prefix, prop, isolated_atom_info_ke | |||
| ) | |||
|
|
|||
|
|
|||
| @click.command("quippy") | |||
There was a problem hiding this comment.
Is this already under "descriptors" somehow? If not, we should possibly move it under such a category, first because quippy also provides potentials, and 2nd, because people might want to calculate ACE descriptors some day.
There was a problem hiding this comment.
Ah yes, this is the reason for this renaming. Currently I've merged what was in "eval" and "descriptors" into "calc", because things in both categories could be "calculated" (@gabor1 pointed out). Didn't think about quippy/ace doing both, properties and descriptors. Maybe instead should rename "eval" -> "calc-prop" & "descriptors" -> "calc-desc" or "calc-properties" & "calc-descriptors"? Just wfl descriptors would be fine, but wfl properties is maybe not as clear? Or should we just keep eval and descriptors? (It's not a big issue, but I thought I'd touch on this in the same PR"
There was a problem hiding this comment.
So this is now "calc quippy"? Can you briefly list the relevant part of the nested command tree structure that's available now (as of this PR)?
There was a problem hiding this comment.
yes; edited the PR description
|
what about moving quippy from "calc" to a new top level category "descriptors"? |
|
I guess the only shortcoming is that descriptors are also "calculated", but if that's fine with you I'll make the change. |
|
On slack @gabor1 suggests longer names, so how about |
|
Done! |
|
Is this ok to merge? The command tree in the initial PR message is what this PR implements. |
|
I'm generally happy with the new names. I do want to check what "atomization-energy" does, and whether it deserves to be its own subcommand. What I assumed it was doing makes more sense as an optional flag to all the other calculator choices. |
|
OK, now I understand what it does - processes already calculated energies for isolated atoms and regular configurations and from those computes the atomization energy for each. If this is correct, @gelzinyte , I'm happy for it to be its own thing (i.e. not an optional flag to a regular calculate like I suggested above), but I'm not sure it belongs in the same top level command as ace/gap/mace, which are really doing a calculation. |
|
@gelzinyte trying to do some cleanup. Do we remember what's the status of this one, esp. with respect to what we described in the paper? |
Mainly create new command
calcwhich houses sub-commands for calculating properties (energy/forces/.. with gap/aca/mace, atomization energy) and descriptors (quippy). I think this is slightly more intuitive. Any objections? Any other suggestions? @bernstei @simonwengertAlso closes #193
The cli options are now