set_output_archive_period broken

[liamhuber]

the set_output_archive_period of protocols is broken.

Minimum Working Example

from pyiron.project import Project
pr = Project("tmp")
pr.remove_jobs(recursive=True);
pr.get_repository_status()
>

Module	Git head
pyiron_contrib	988c5e6
pyiron_mpie	011c8cdae3237d7c6dcd4e81a5624560ead0f32a
pyiron	392ca25

md = pr.create_job(pr.job_type.MolecularDynamics, 'md')
md.set_output_archive_period(2)
md.whitelist  # the tree doesn't format nicely in markdown
>
{'initial_velocity': {'input': {}, 'output': {}},
 'initial_forces': {'input': {}, 'output': {}},
 'check_steps': {'input': {}, 'output': {}},
 'clock': {'input': {}, 'output': {'n_counts': 2}},
 'calc_static': {'input': {}, 'output': {'energy_pot': 1, 'forces': 1000}},
 'verlet_positions': {'input': {}, 'output': {'positions': 1000}},
 'verlet_velocities': {'input': {},
  'output': {'energy_kin': 1, 'velocities': 1000}}}

I think the crux of the issue is that at protocol.generic.Vertex._set_archive_period we loop over Vertex.output to look for keys to use in setting the archive period, but prior to running the output is almost empty.

I'm planning to play around with giving the vertices predefined input and output fields wherever the children of the Vertex ABC are defined. As a side effect this would directly solve this problem.

The only other solution I see at the moment is to do some sort of 'ghost run' to spoof the output of all the vertices, store the fields, and then use those. This is complicated though, and I'd rather not have _set_archive_period than have a solution that muddy.

[jan-janssen]

Is this related to pyiron/pyiron#488 ?

[liamhuber]

@jan-janssen, conceptually, yes. We allow individual dump frequencies for each input/output attribute; the linked issue allows either the base rate or no dumping. There's also some difference in motivation, where I think Sam was concerned with speed whereas we're more concerned with memory. But both share the principle of saving only the data the user really wants to save.

The fact that the particular method set_output_archive_period is bugged is completely unrelated though. The bug here is because the output dictionaries are initially (almost) empty, so when we try to loop over their keys nothing happens. set_input_archive_period doesn't have the same problem because the input dictionaries get (mostly) filled at an earlier stage.

My plan for fixing this is eventually to move to a Vertex-as-box model where the vertex sub-classes define input and output fields which then exist from instantiation. I'm still open to alternative architectures though, and implementing that plan is a ways off at the moment.

[jan-janssen]

@liamhuber Is this issue fixed?

[liamhuber]

@liamhuber Is this issue fixed?

Afaik it's still a problem; @raynol-dsouza did you patch it at any point?

[raynol-dsouza]

I believe it is fixed.

[liamhuber]

I believe it is fixed.

Could you please try out the mwe above to confirm and then close? If it is still broken, imo there is no need to fix before Christmas

[raynol-dsouza]

I was wrong; set_output_archive_period is still broken, at least on the master branch. I believe I fixed this issue in the branch external_ham, but that branch has not been updated in a while, and is throwing errors which it wasn't throwing before. I can take a look at it again after the holidays.