search

stfc / janus-core

Tools for machine learnt interatomic potentials
https://stfc.github.io/janus-core/
BSD 3-Clause "New" or "Revised" License
9 stars 7 forks source link

Rename `vectors_only` #180

Closed ElliottKasoar closed 3 weeks ago

ElliottKasoar commented 2 months ago 
          `vectors_only` isn't the clearest name to me as to what that means. The help explains it, but I think `cell_only` would possibly be better since the atomic positions are also vectors.

_Originally posted by @oerc0122 in https://github.com/stfc/janus-core/pull/179#discussion_r1624393815_

ElliottKasoar commented 2 months ago

Last suggestion was cell_lengths, although my only comment would be it's not particularly clear that it's a boolean anymore. I'd read it as a number to input.

ElliottKasoar commented 3 weeks ago

We've had another comment that --vectors-only is confusing, as it suggests that the atomic positions don't change.

Given that the help mentions both ("Optimize cell vectors, as well as atomic positions.") my feeling is the "only" is the misleading part.

Following various discussions, I'm proposing --opt-cell-lengths, and potentially renaming --fully-opt to --opt-fully for consistency.

--fully-opt/--opt-fully is arguably ambiguous too, but I don't think it's misleading in the same way, so I would hope if in doubt a user would be more inclined to check the help, rather than making a wrong assumption as seems more likely with --vectors-only.