search

aiidaplugins / aiida-lammps

LAMMPS plugin for AiiDA
https://aiida-lammps.readthedocs.io
MIT License
22 stars 14 forks source link

Naming of `CalcJob` plugins #56

Closed sphuber closed 1 year ago

sphuber commented 1 year ago

Currently, the calcjob plugins do not contains Lammps in the class name. This is not a problem per se, but since the process_label attribute is taken from the class name, it currently is not very clear. The verdi process list output for example now contains ForceCalculation, MdCalculation, etc. It is not immediately clear that these are LAMMPS calculations.

Ideally, I think these are called LammpsForceCalculation, LammpsMdCalculation, etc.

On this note, I think that BaseLammpsCalculation would better be named LammpsBaseCalculation for consistency. It would also distinguish it from the aiida_lammps.calculations.lammps.BaseLammpsCalculation. I was quite confused to see the same class name for different implementations.

Of course this would be backwards-incompatible if users are importing these directly, but given that they are probably using the entry points, this shouldn't be too much of a problem.

sphuber commented 1 year ago

pinging @JPchico @chrisjsewell

JPchico commented 1 year ago

I see no problem with changing BaseLammpsCalculation for LammpsBaseCalculation, I think that it makes sense. As I wrote in #55 the idea is to change the ForceCalculation, MdCalculation for Workchains. I see no problem with renaming them.

This will be of course a backwards-incompatible changes, but considering all the changes that are being made to make the plugin aiida-2.x compatible and so on, I think it is acceptable.

JPchico commented 1 year ago

Closed via 186afeb