Skip to content

Commit

Permalink
[minor] Introduce for-loop (#309)
Browse files Browse the repository at this point in the history
* Change paradigm to whether or not the node uses __reduced__ and a constructor

Instead of "Meta" nodes

* Allow direct use of Constructed children

* Move and update constructed stuff

* Add new singleton behaviour so factory-produced classes can pass is-tests

* PEP8 newline

* Remove unnecessary __getstate__

The object isn't holding instance level state and older versions of python bork here.

* Add constructed __*state__ compatibility for older versions

* 🐛 add missing `return`

* Format black

* Revert singleton

* Remove constructed

It's superceded by the snippets.factory stuff

* Format black

* Let the factory clear method take specific names

* Don't override __module__ to the factory function

If it was explicitly set downstream, leave that. But if the user left it empty, still default it back to the factory function's module

* Clean up storage if job tests fail

* Make tinybase the default storage backend

* Switch Function and Macro over to using classfactory

With this, everything is pickleable (unless you slap something unpickleable on top, or define it in a place that can't be reached by pickle like inside a local function scope). The big downside is that `h5io` storage is now basically useless, since all our nodes come from custom reconstructors. Similarly, for the node job `DataContainer` can no longer store the input node. The `tinybase` backend is still working ok, so I made it the default, and I got the node job working again by forcing it to cloudpickle the input node on saving. These are some ugly hacks, but since storage is an alpha feature right now anyhow, I'd prefer to push ahead with pickleability.

* Remove unused decorator

And reformat tests in the vein of usage in Function and Macro

* Format black

* Expose concurrent.futures executors on the creator

* Only expose the base Executor from pympipool

Doesn't hurt us now and prepares for the version bump

* Extend `Runnable` to use a non-static method

This is significant. `on_run` is no longer a property returning a staticmethod that will be shipped off, but we directly ship off `self.on_run` so `self` goes with it to remote processes. Similarly, `run_args` gets extended to be `tuple[tuple, dict]` so positional arguments can be sent too.

Stacked on top of pickleability, this means we can now use standard `concurrent.futures.ProcessPoolExecutor` -- as long as the nodes are all defined somewhere importable, i.e. not in `__main__`. Since working in notebooks is pretty common, the more flexible `pympipool.Executor` is left as the default `Workflow.create.Executor`.

This simplifies some stuff under the hood too, e.g. `Function` and `Composite` now just directly do their thing in `on_run` instead of needing the misdirection of returning their own static methods.

* Format black

* Expose concurrent.futures executors on the creator

* Only expose the base Executor from pympipool

Doesn't hurt us now and prepares for the version bump

* Extend `Runnable` to use a non-static method

This is significant. `on_run` is no longer a property returning a staticmethod that will be shipped off, but we directly ship off `self.on_run` so `self` goes with it to remote processes. Similarly, `run_args` gets extended to be `tuple[tuple, dict]` so positional arguments can be sent too.

Stacked on top of pickleability, this means we can now use standard `concurrent.futures.ProcessPoolExecutor` -- as long as the nodes are all defined somewhere importable, i.e. not in `__main__`. Since working in notebooks is pretty common, the more flexible `pympipool.Executor` is left as the default `Workflow.create.Executor`.

This simplifies some stuff under the hood too, e.g. `Function` and `Composite` now just directly do their thing in `on_run` instead of needing the misdirection of returning their own static methods.

* Format black

* Compute qualname if not provided

* Fail early if there is a <locals> function in the factory made hierarchy

* Skip the factory fanciness if you see <locals>

This enables _FactoryMade objects to be cloudpickled, even when they can't be pickled, while still not letting the mere fact that they are dynamic classes stand in the way of pickling.

Nicely lifts our constraint on the node job interaction with pyiron base, which was leveraging cloudpickle

* Format black

* Test ClassFactory this way too

* Test existing list nodes

* Rename length base class

* Refactor transformers to use on_run and run_args more directly

* Introduce an inputs-to-dict transformer

* Preview IO as a separate step

To guarantee IO construction happens as early as possible in case it fails

* Add dataframe transformer

* Remove prints 🤦

* Add dataframe transformer tests

* Add transformers to the create menu

* Format black

* 🧹 be more consistent in caching/shortcuts

Instead of always defining private holders by hand

* Introduce a dataclass node

* Give the dataclass node a simpler name

Since we can inject attribute access, I don't anticipate ever needing the reverse dataclass-to-outputs node, so let's simplify the naming here.

* Remove unused import

* Set the output type hint automatically

* Add docs

* Add tests

* Format black

* PEP8 newline

* Introduce for-loop

* Refactor: break _build_body into smaller functions

* Resolve dataframe column name conflicts

When a body node has the same labels for looped input as for output

* Update docstrings

* Refactor: rename file

* Don't break when one of iter or zipped is empty

* 🐛 pass body hint, not hint and default, to row collector input hint

* Silence disconnection warning

Since(?) disconnection is reciprocal it was firing left right and centre

* Update deepdive

* Remove old for loop

* Remove unused import

* Add tests

* Remove unused attributes

* Add a shortcut for assigning an executor to all body nodes

* Format black

* Format black

---------

Co-authored-by: pyiron-runner <pyiron@mpie.de>
  • Loading branch information
liamhuber and pyiron-runner committed May 6, 2024
1 parent c022822 commit 8833f5b
Show file tree
Hide file tree
Showing 7 changed files with 1,364 additions and 349 deletions.
760 changes: 546 additions & 214 deletions notebooks/deepdive.ipynb

Large diffs are not rendered by default.

6 changes: 0 additions & 6 deletions pyiron_workflow/channels.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import typing
from abc import ABC, abstractmethod
import inspect
from warnings import warn

from pyiron_workflow.has_interface_mixins import HasChannel, HasLabel, UsesState
from pyiron_workflow.has_to_dict import HasToDict
Expand Down Expand Up @@ -172,11 +171,6 @@ def disconnect(self, *others: Channel) -> list[tuple[Channel, Channel]]:
self.connections.remove(other)
other.disconnect(self)
destroyed_connections.append((self, other))
else:
warn(
f"The channel {self.label} was not connected to {other.label}, and"
f"thus could not disconnect from it."
)
return destroyed_connections

def disconnect_all(self) -> list[tuple[Channel, Channel]]:
Expand Down
10 changes: 8 additions & 2 deletions pyiron_workflow/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ def __init__(self):
# this if-clause and just letting users of python <3.10 hit an error.
self.register("pyiron_workflow.node_library.standard", "standard")

@property
@lru_cache(maxsize=1)
def for_node(self):
from pyiron_workflow.for_loop import for_node

return for_node

@property
@lru_cache(maxsize=1)
def macro_node(self):
Expand All @@ -82,12 +89,11 @@ def Workflow(self):
@lru_cache(maxsize=1)
def meta(self):
from pyiron_workflow.transform import inputs_to_list, list_to_outputs
from pyiron_workflow.loops import for_loop, while_loop
from pyiron_workflow.loops import while_loop
from pyiron_workflow.snippets.dotdict import DotDict

return DotDict(
{
for_loop.__name__: for_loop,
inputs_to_list.__name__: inputs_to_list,
list_to_outputs.__name__: list_to_outputs,
while_loop.__name__: while_loop,
Expand Down
Loading

0 comments on commit 8833f5b

Please sign in to comment.