Save and load don't work intuitively

[ligerzero-ai]

Consider:

inp = CalcInputMinimize()
inp.relax_cell = False
Engine = LammpsEngine(EngineInput = inp)

Engine.working_directory = "test_opt_cubic_cell"
# Adjust this line to match your LAMMPS installation
# If you are using conda-lammps or a LAMMPS binary on your system, you can use directly:
Engine.command = "lmp -in in.lmp -log minimize.log"
Engine.lammps_log_filepath = "minimize.log"
Engine.input_script_pair_style = "eam/fs"
Engine.path_to_model = "Al-Fe_eam.fs"
# /root/github_dev/pyiron_workflow_atomistics/pyiron_workflow_atomistics
structure = bulk("Fe", a=2.828, cubic=True)
wf1 = Workflow(Engine.working_directory, delete_existing_savefiles=True)
wf1.opt_cubic_cell = optimise_cubic_lattice_parameter(
    structure=structure,
    name="Fe",
    crystalstructure="bcc",
    calculation_engine=Engine,
    parent_working_directory="opt_cubic_cell",
    rattle=0.1,
    strain_range=(-0.02, 0.02),
    num_points=6,
)
wf1.run()
wf1.save(filename="opt_cubic_cell_wf")

When I try to load the workflow that I saved,

import os
wf2 = Workflow(label=f"opt_cubic_cell_wf")
print(wf2.outputs)

it provides empty output

print(wf2.outputs)
OutputsWithInjection []

Or fails directly when I invoke the wf.load() method:

wf2 = Workflow.load(filename="opt_cubic_cell_wf")
wf2

Instead, it is expected (?) that you invoke Workflow("exact same label here")

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[36], line 1
----> 1 wf2 = Workflow.load(filename="opt_cubic_cell_wf")
      2 wf2

TypeError: Node.load() missing 1 required positional argument: 'self'

Expected behaviour (imho) should be that it loads the workflow from the savefile specified

[ligerzero-ai]

So basically a really long way to say that save and load should behave symmetrically. It's unintuitive that they don't

[liamhuber]

When I try to load the workflow that I saved,

import os
wf2 = Workflow(label=f"opt_cubic_cell_wf")
print(wf2.outputs)

it provides empty output

print(wf2.outputs)
OutputsWithInjection []

That's just a full-on bug. Good find. When we call wf.save() it save is to "./{workflow.label}/picklestorage.pckl", but when we call wf.save(filename="something") it saves it to "./something.pckl" instead of "./something/picklestorage.pckl".

It's possible we always want to be serializing directly to "." and not reserving the directory of the workflow's name for anything, but good or bad, that's a design choice. That save is sending stuff with a filename set to "." is fully a bug.

Or fails directly when I invoke the wf.load() method:

wf2 = Workflow.load(filename="opt_cubic_cell_wf")
wf2

Instead, it is expected (?) that you invoke Workflow("exact same label here")

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[36], line 1
----> 1 wf2 = Workflow.load(filename="opt_cubic_cell_wf")
      2 wf2

TypeError: Node.load() missing 1 required positional argument: 'self'

Ah, this is a little more subtle. You're not actually invoking wf.load(), you're trying to use .load as a class-method on the Workflow class. That's not an unreasonable thing to do, but it's not symmetric with save (which can only ever make sense on an instance anyhow) and simply not implemented. This is why you get the self error.

So, yes, to reload a workflow you are expected to give it the same name as a saved workflow and let it auto-load (which is working fine) or instantiate it with whatever name you want and then call .load(filename=...) (which is broken with what looks like the exact same bug as .save(filename=...) is experiencing).

I'm not convinced that adding the class method Workflow.load is necessary since it's equivalent to Workflow.__init__ with auto-loading turned on. But I'll try to patch the bug in save load soon such that you can do something like this:

import pyiron_workflow as pwf

wf = pwf.Workflow("some_label")
...
wf.save(filename="this_is_not_the_label")

wf2 = pwf.Workflow("five_minutes_later")
wf2.load(filename="this_is_not_the_label")

# or just:
# wf2 = pwf.Workflow("this_is_not_the_label")

[ligerzero-ai]

Would loading a specific file on a path i.e. "/some/path/here/wf.pckl" work too? Can you make sure that that works?

The reason:

Consider a high throughput study that contains n-jobs that have been reasonably divided up into n sub-workflows.

It's somewhat clunky to have to load each one of the jobs with a os.chdir command before invoking the loading of each one.

[liamhuber]

Yeah, right now the "filename" is actually referring to a directory path. We can make this compatible with the use case you're talking about, but some details might look a little funky. I need to think a bit about how this should interact with the idea of nodes in the graph being able to convert their lexical path to a file path and make sure we don't accidentally do some malicious design.

At any rate, for sure we can make it doable without an explicit chdir