search

pyiron / pyiron_atomistics

pyiron_atomistics - an integrated development environment (IDE) for atomistic simulation in computational materials science.
https://pyiron-atomistics.readthedocs.io
BSD 3-Clause "New" or "Revised" License
39 stars 15 forks source link

Should master job set `calc_minimize`? #1441

Closed samwaseda closed 1 month ago

samwaseda commented 1 month ago

https://github.com/pyiron/pyiron_atomistics/blob/233478c295fe2d67aeffb3e2d1a16a4160aa67ad/pyiron_atomistics/atomistics/master/elasticmatrix.py#L127

I saw this line and was a little bit upset, because from my point of view, it's up to the child job to decide whether the atoms should be relaxed or not, i.e. it should be:

job = pr.create.job.Lammps("job")
job.structure = structure
job.calc_minimize()
master = job.create_job("ElasticMatrixJob", "elastic")
master.run()

From the very fundamental point of view, I support this structure, because it's the running mode of the child job, but also because there's only minimise yes or no from the master job, while there is a whole range of parameters that should be adjusted in the minimisation. So I'd like to remove it because I found it super confusing.

jan-janssen commented 1 month ago

To me the calc_minimize() is similar to the setting of the interatomic potential so it makes sense to configure it on the child job level. Still as calc_minimize() is a generic function I am also fine with having the option on the master to set the calc_minimize() of the child job.

samwaseda commented 1 month ago

So can I remove it from ElasticMatrixJob?

jan-janssen commented 1 month ago

So can I remove it from ElasticMatrixJob?

Yes, that sounds like a reasonable suggestion to me.

samwaseda commented 1 month ago

Resolved here