Wrapped executor naming?

[liamhuber]

@jan-janssen, I forgot to check during the meeting today -- what do you think of the wrapped executor name pyiron_workflow.NodeSlurmExecutor?

I do want to keep a bespoke executor so that all the serialization location and cleanup can be managed, and I internally add silent checks to verify and indicate that this for usage with pyiron_workflow nodes, so I'm content with the behaviour. But I want to check with you (a) whether the use of the term Node in a new context is too confusing, and (b) whether you want any more explicit attribution to executorlib right in the name?

I think it's nice for usability if Slurm and Executor appear in the class name, but am otherwise completely flexible.

[jan-janssen]

The challenging part are the two executors for SLURM, executorlib has the SlurmClusterExecutor https://executorlib.readthedocs.io/en/latest/2-hpc-cluster.html#slurm and the SlurmJobExecutor https://executorlib.readthedocs.io/en/latest/3-hpc-job.html#slurm . How do you differentiate those two? Are they both named NodeSlurmExecutor?

[liamhuber]

Currently, I only wrap the SlurmClusterExecutor. I also wrap and test the SingleNodeExecutor and TestClusterExecutor, but I don't (and don't currently intend to) expose these two to users in the API.

In all cases the wrapping follows the super simple pattern class PWFClass(CacheOverride, ExecutorlibClass). It's possible the SlurmJobExecutor will be able to follow the same super-simple pattern, I just haven't had time to test this.

With an eye to exposing other executorlib executors, what about {PREFIX}SlurmClusterExecutor? For the prefix I could imagine Wrapped, Executorlib, Workflow, etc., but don't have strong feelings.

[jan-janssen]

I continue to think about it and I wonder, would it be possible to dynamically generate the executor classes in pyiron_workflow? The advantages I see:

• Users could update their workflow by switching from one executor to the next, e.g. from ProcessPoolExecutor to SlurmClusterExecutor.
• If another framework exists which also uses the executorlib pattern users could combine it with pyiron_workflow, here are a couple of similar frameworks I am aware off [Documentation] Collect similar tools executorlib#630
• It would be possible to define executorlib as optional dependency for pyiron_workflow, so users who do not need the HPC integration, e.g. experimentalists , would benefit from a smaller installation foot print.

[liamhuber]

Yes, absolutely. It's not only possible, it's super easy. I drafted an example in #726 and you can see it plays fine with the tests. Use case changes would be something like this:

import pyiron_workflow as pwf

n = pwf.std.Sleep(60)
n.executor = (pwf.NodeSlurmExecutor, (), {"resource_dict": {"partition": "s.cmfe"}})
n.run()

becomes

import executorlib
import pyiron_workflow as pwf

n = pwf.std.Sleep(60)
n.executor = (pwf.wrapped_executorlib, (executorlib.SlurmClusterExecutor,), {"resource_dict": {"partition": "s.cmfe"}})
n.run()

• Users could update their workflow by switching from one executor to the next, e.g. from ProcessPoolExecutor to SlurmClusterExecutor.

Non-file-based executors don't need to be wrapped, you can just directly write n.executor = (ProcessPoolExecutor, (), {}) (or use a plain ProcessPoolExecutor instance). The wrapping here is just to make sure that the cache files get written somewhere nice and unique, and so the node knows where to go to automatically clean up the cache after recovering the output. There is no barrier in principle to using pyiron_workflow with plan executorlib executors, it's just much less convenient because you need to always provide cache keys and need to manually clean up completed calculations. The wrapping is 100% for (a lot of!) convenience.

• If another framework exists which also uses the executorlib pattern users could combine it with pyiron_workflow, here are a couple of similar frameworks I am aware off

Yes, if it followed the executorlib pattern down to the argument names (at least the resource_dict/cache_directory/cache_key) and file writing (*_i/o.h5), it would work.

I agree that this is an advantage, but I do see it as a small one since if they had such a class they could just do

import pyiron_workflow as pwf
from pyiron_workflow.executors.wrapped_executorlib import CacheOverride

from my_module import MyExecutorlibLikeExecutor

class MyWrappedExecutor(CacheOverride, MyExecutorlibLikeExecutor):
    pass

n = pwf.std.Sleep(60)
n.executor = (MyWrappedExecutor, (), {})
n.run()

I fully agree this is less nice than (pwf.wrapped_executorlib, (MyExecutorlibLikeExecutor,), {}), but the magnitude of improvement multiplied by the number of times I expect it to be relevant is a small thing.

*It would be possible to define executorlib as optional dependency for pyiron_workflow, so users who do not need the HPC integration, e.g. experimentalists , would benefit from a smaller installation foot print.

Yes, it's already an optional install.

I see two main disadvantages to dynamically generating subclasses:

1. The Number 1 complaint I get about pyiron_workflow is from people who look under the hood and complain that it's too complicated. I worry (more!) dynamic subclasses will not help this.
2. This wrapping necessarily makes assumptions about how the wrapped class behaves; currently I directly test all the executors I wrap and provide, so it's guaranteed to work. If suggest that the main way to accomplish this is by passing an executor class of your choice to a wrapping function, I would want to make sure that we fail very cleanly when they give one that isn't going to work. This is probably possible, but I don't want to implement such a check.

[jan-janssen]

I definitely agree with complain one and we had the same discussion in executorlib pyiron/executorlib#703 . The integration of an Executor is not an easy task, so it is highly likely that the internal structure is complex as a result. Maybe one idea would be instead of:

import executorlib
import pyiron_workflow as pwf

n = pwf.std.Sleep(60)
n.executor = (pwf.wrapped_executorlib, (executorlib.SlurmClusterExecutor,), {"resource_dict": {"partition": "s.cmfe"}})
n.run()

something like:

import executorlib
import pyiron_workflow as pwf

n = pwf.std.Sleep(60)
n.executor = pwf.executor.executorlib.SlurmClusterExecutor(resource_dict={"partition": "s.cmfe"})
n.run()

This would make the Executor feel like a node and users could choose between different libraries which provide executors.

[liamhuber]

Yes, you can already¹ do

import executorlib
import pyiron_workflow as pwf

n = pwf.std.Sleep(60)
n.executor = pwf.executors.wrapped_executorlib.NodeSlurmExecutor(resource_dict={"partition": "s.cmfe"})
n.run()

However, then you are responsible for managing the executor shutdown, etc. The exact same is true if you do n.executor = concurrent.futures.ProcessPoolExecutor(). So I don't know about it feeling like a node, but users are already free to use executors in this way from any library they like that provides a concurrent.futures._base.Executor interface. If that executor does funky stuff with the file system, they're responsible for taking care of that, and the pwf.executors.wrapped_executorlib module is just me giving executorlib preferential treatment and doing that management in a fully-automated way on the user's behalf.

The tuple-formulation is for storing instructions on how to build an executor rather than an executor itself. If that's what's concerning you, it's a separate issue. We can't (generally?) serialize a live executor instance, so providing instead the instructions on how to build one lets us include it as part of the save file, and even nest executor assignments under some circumstances (concurrent.futures.ThreadPoolExecutor and pwf.executors.wrapped_executorlib.NodeSlurmExecutor both nest fine in this way, concurrent.futures.ProcessPoolExecutor currently does not).

I'd like to offer less verbosity to the user by extending the interface for what is a permissible set of instructions (cf. #715), e.g. n.executor = concurrent.futures.ThreadPoolExecutor, or n.executor = (pwf.NodeSlurmExecutor, {"resource_dict": {"partition": "s.cmfe"}}) instead of always demanding the full tuple[Callable, tuple, dict] triple every time. What I don't want to do make it so pwf.executors.wrapped_executorlib.NodeSlurmExecutor(resource_dict={"partition": "s.cmfe"}) looks like it's returning an executor instance but is actually returning a set of instructions. IMO it's better to be clear that you're allowed to provide and instance or instructions but to keep it clear which one is happening.

Footnotes

1. e.g. here in the tests with a ProcessPool inside a with context for managing the shutdown or here again with a process pool where I explicitly shutdown the executor later. ↩