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
44 stars 15 forks source link

SphinxBase.set_occupancy_smearing is broken #628

Open pmrv opened 2 years ago

pmrv commented 2 years ago

When using set_occupancy_smearing on a sphinx job, the smearing parameters are correctly saved to job.input, but not to job.input.sphinx, which causes them to be ignored when the job runs. This probably also causes #627.

I see two ways to fix this, either set_occupancy_smearing writes directly to job.input.sphinx or both are synchronized before the job runs proper. Any preference @samwaseda ?

samwaseda commented 2 years ago

I see two ways to fix this, either set_occupancy_smearing writes directly to job.input.sphinx or both are synchronized before the job runs proper. Any preference @samwaseda ?

This is actually a super important question to be asked in larger audience. Before @ashtonmv implemented Group, I made it intentionally impossible to change data in what's now job.input.sphinx, because I couldn't guarantee that everything would be synchronised. So job.input.sphinx was essentially created only when run was called, and only the whole string was stored (which means the user had to manipulate the string). Now, the behaviour is relatively similar, meaning job.input.sphinx is meant to be created only when run is called, but job.input.sphinx can be also created if the user wishes to do so. Once it's created, it has priority, meaning whatever is written in job.input is not reflected anymore, regardless of what the user does. So this is where the problem actually comes from. I have the feeling that whatever we change in set_ and job.input should be written to both job.input and job.input.sphinx, but at least it's not how things work right now, and I'm not really sure if things will be synchronised properly this way.