From 3f1d9650b86716c0a82e3da4691667fa86c107cc Mon Sep 17 00:00:00 2001 From: liamhuber Date: Wed, 24 Jul 2024 10:15:51 -0700 Subject: [PATCH] [minor] Don't have a default `Executor` Make the user choose one specifically. --- notebooks/deepdive.ipynb | 2 +- notebooks/quickstart.ipynb | 2 +- pyiron_workflow/create.py | 4 ---- tests/integration/test_workflow.py | 2 +- tests/unit/nodes/test_macro.py | 2 +- tests/unit/test_node.py | 8 +++----- tests/unit/test_workflow.py | 8 ++++---- 7 files changed, 11 insertions(+), 17 deletions(-) diff --git a/notebooks/deepdive.ipynb b/notebooks/deepdive.ipynb index b1eda64be..bed19c32a 100644 --- a/notebooks/deepdive.ipynb +++ b/notebooks/deepdive.ipynb @@ -3883,7 +3883,7 @@ "\n", "You can currently run nodes in different process by setting that node's `executor` to an instance of a compliant executor object -- i.e. that takes the standard `submit` method of `concurrent.futures.Executor`, returns a `concurrent.futures.Future` (or sub-class). The built-in `Node` instances (workflows, macros, function nodes, etc.) are `pickle`-compliant, and thus will work with a standard `ProcessPoolExecutor` or `ThreadPoolExecutor` from `concurrent.futures`.\n", "\n", - "For the case of `ProcessPoolExecutor` (the default `Executor` on the creator), the other process needs to be able to find an import the nodes, so they can't have been created in `__main__` (e.g. here in notebook) but need to be in some importable `.py` file. You might also want to have a node holding un-pickleable data. For these cases, we make a couple more flexible executors available on the creator. There is a toy `CloudpickleProcessPoolExecutor` which is a minimal example of handling dynamically defined/un-picklable data and useful for learning, but we also link to `executorlib.Executor`, which is both flexible and powerful. For compatibility with GitHub CI on macos, this currently also requires an extra keyword.\n", + "For the case of `ProcessPoolExecutor`, the other process needs to be able to find an import the nodes, so they can't have been created in `__main__` (e.g. here in notebook) but need to be in some importable `.py` file. You might also want to have a node holding un-pickleable data. For these cases, we make a couple more flexible executors available on the creator. There is a toy `CloudpickleProcessPoolExecutor` which is a minimal example of handling dynamically defined/un-picklable data and useful for learning, but we also link to `executorlib.Executor`, which is both flexible and powerful. For compatibility with GitHub CI on macos, this currently also requires an extra keyword.\n", "\n", "Here's a simple example of executor usage:" ] diff --git a/notebooks/quickstart.ipynb b/notebooks/quickstart.ipynb index 141eade54..5e404655a 100644 --- a/notebooks/quickstart.ipynb +++ b/notebooks/quickstart.ipynb @@ -1775,7 +1775,7 @@ "To learn more, take a look at the `deepdive.ipynb` notebook, and/or start looking through the class docstrings. Here's a brief map of what you're still missing:\n", "\n", "- Distributing node execution onto remote processes\n", - " - Single core parallel python processes is available by setting the `.executor` attribute to a compatible executor instance, e.g. `Workflow.create.Executor()`\n", + " - Parallel computation is available by setting the `.executor` attribute to a compatible executor instance, e.g. `Workflow.create.ProcessPoolExecutor()`\n", "- Acyclic graphs\n", " - Execution for graphs whose data flow topology is a DAG happens automatically, but you're always free to specify this manually with `Signals`, and indeed _must_ specify the execution flow manually for cyclic graphs -- but cyclic graphs _are_ possible!\n", "- Complex flow nodes\n", diff --git a/pyiron_workflow/create.py b/pyiron_workflow/create.py index 305b05bbf..d5409848e 100644 --- a/pyiron_workflow/create.py +++ b/pyiron_workflow/create.py @@ -24,9 +24,6 @@ if TYPE_CHECKING: from pyiron_workflow.node_package import NodePackage -# Specify the standard executor -Executor = ProcessPoolExecutor - class Creator(metaclass=Singleton): """ @@ -47,7 +44,6 @@ def __init__(self): self._package_access = DotDict() self._package_registry = bidict() - self.Executor = Executor # Standard lib self.ProcessPoolExecutor = ProcessPoolExecutor self.ThreadPoolExecutor = ThreadPoolExecutor diff --git a/tests/integration/test_workflow.py b/tests/integration/test_workflow.py index 2b6bd02eb..a87ab335a 100644 --- a/tests/integration/test_workflow.py +++ b/tests/integration/test_workflow.py @@ -138,7 +138,7 @@ def test_executor_and_creator_interaction(self): wf.register("static.demo_nodes", "demo") wf.before_pickling = wf.create.demo.OptionallyAdd(1) - wf.before_pickling.executor = wf.create.Executor() + wf.before_pickling.executor = wf.create.ProcessPoolExecutor() wf() wf.before_pickling.future.result(timeout=120) # Wait for it to finish wf.executor_shutdown() diff --git a/tests/unit/nodes/test_macro.py b/tests/unit/nodes/test_macro.py index ae9dbba06..21433a9ec 100644 --- a/tests/unit/nodes/test_macro.py +++ b/tests/unit/nodes/test_macro.py @@ -212,7 +212,7 @@ def test_with_executor(self): # at the downstream output, and none of this is happening in a workflow original_one = macro.one - macro.executor = macro.create.Executor() + macro.executor = macro.create.ProcessPoolExecutor() self.assertIs( NOT_DATA, diff --git a/tests/unit/test_node.py b/tests/unit/test_node.py index 1bed1209c..bb7123b84 100644 --- a/tests/unit/test_node.py +++ b/tests/unit/test_node.py @@ -1,4 +1,4 @@ -from concurrent.futures import Future +from concurrent.futures import Future, ProcessPoolExecutor import os import sys import unittest @@ -6,8 +6,6 @@ from pyiron_snippets.files import DirectoryObject from pyiron_workflow.channels import InputData, NOT_DATA - -from pyiron_workflow.create import Executor from pyiron_workflow.mixin.injection import OutputDataWithInjection, OutputsWithInjection from pyiron_workflow.io import Inputs from pyiron_workflow.node import Node @@ -143,7 +141,7 @@ def test_check_readiness(self): ) def test_force_local_execution(self): - self.n1.executor = Executor() + self.n1.executor = ProcessPoolExecutor() out = self.n1.run(force_local_execution=False) with self.subTest("Test running with an executor fulfills promises"): self.assertIsInstance( @@ -179,7 +177,7 @@ def test_force_local_execution(self): "happens" ) - self.n2.executor = Executor() + self.n2.executor = ProcessPoolExecutor() self.n2.inputs.x = 0 self.assertEqual( 1, diff --git a/tests/unit/test_workflow.py b/tests/unit/test_workflow.py index a12b1b1fd..053e0b9a8 100644 --- a/tests/unit/test_workflow.py +++ b/tests/unit/test_workflow.py @@ -197,7 +197,7 @@ def test_with_executor(self): wf.b = wf.create.function_node(plus_one, x=wf.a) original_a = wf.a - wf.executor = wf.create.Executor() + wf.executor = wf.create.ProcessPoolExecutor() self.assertIs( NOT_DATA, @@ -243,7 +243,7 @@ def test_parallel_execution(self): wf.fast = five() wf.sum = sum(a=wf.fast, b=wf.slow) - wf.slow.executor = wf.create.Executor() + wf.slow.executor = wf.create.ProcessPoolExecutor() wf.slow.run() wf.fast.run() @@ -419,7 +419,7 @@ def add_three_macro(self, one__x): msg="Sanity check, pulling here should work perfectly fine" ) - wf.m.one.executor = wf.create.Executor() + wf.m.one.executor = wf.create.ProcessPoolExecutor() with self.assertRaises( ValueError, msg="Should not be able to pull with executor in local scope" @@ -428,7 +428,7 @@ def add_three_macro(self, one__x): wf.m.one.executor_shutdown() # Shouldn't get this far, but if so, shutdown wf.m.one.executor = None - wf.n1.executor = wf.create.Executor() + wf.n1.executor = wf.create.ProcessPoolExecutor() with self.assertRaises( ValueError, msg="Should not be able to pull with executor in parent scope"