From d0c6676c361ec3b5846ed4fb44f9d4dfcb07d495 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Thu, 12 Oct 2023 09:53:05 -0700 Subject: [PATCH] Reproduce and split up pyiron_workflow executor tests --- tests/unpicklable_elements/__init__.py | 21 ++++++ tests/unpicklable_elements/test_args.py | 69 ++++++++++++++++++ tests/unpicklable_elements/test_callable.py | 74 ++++++++++++++++++++ tests/unpicklable_elements/test_exception.py | 55 +++++++++++++++ tests/unpicklable_elements/test_return.py | 74 ++++++++++++++++++++ tests/unpicklable_elements/test_timeout.py | 67 ++++++++++++++++++ 6 files changed, 360 insertions(+) create mode 100644 tests/unpicklable_elements/__init__.py create mode 100644 tests/unpicklable_elements/test_args.py create mode 100644 tests/unpicklable_elements/test_callable.py create mode 100644 tests/unpicklable_elements/test_exception.py create mode 100644 tests/unpicklable_elements/test_return.py create mode 100644 tests/unpicklable_elements/test_timeout.py diff --git a/tests/unpicklable_elements/__init__.py b/tests/unpicklable_elements/__init__.py new file mode 100644 index 00000000..7411989d --- /dev/null +++ b/tests/unpicklable_elements/__init__.py @@ -0,0 +1,21 @@ +""" +`pympipool` should be able to handle the case where _no_ elements of the execution can +be pickled with the traditional `pickle` module but rather require `cloudpickle`. + +This is particularly important for compatibility with `pyiron_workflow`, which +dynamically defines (unpickleable) all sorts of objects. + +Currently, `pyiron_workflow` defines its own executor, +`pyiron_workflow.executors.CloudPickleProcessPool`, which can handle these unpickleable + things, but is otherwise very primitive compared to + `pympipool.mpi.executor.PyMPISingleTaskExecutor`. + +Simply replacing `CloudPickleProcessPool` with `PyMPISingleTaskExecutor` in the +`pyiron_atomistics` tests mostly works OK, and work perfectly when the tests are ported +to a notebook, but some tests hang indefinitely on CI and running unittests locally. + +To debug this, we break the tests up into their individual components (so hanging +doesn't stop us from seeing the results of other tests). Once everything is running, +these can be re-condensed into a single test file and this entire subdirectory can be +deleted. +""" diff --git a/tests/unpicklable_elements/test_args.py b/tests/unpicklable_elements/test_args.py new file mode 100644 index 00000000..241c4e53 --- /dev/null +++ b/tests/unpicklable_elements/test_args.py @@ -0,0 +1,69 @@ +from functools import partialmethod +from time import sleep +import unittest + +from pympipool.mpi.executor import PyMPISingleTaskExecutor + + +class Foo: + """ + A base class to be dynamically modified for testing CloudpickleProcessPoolExecutor. + """ + def __init__(self, fnc: callable): + self.fnc = fnc + self.result = None + + @property + def run(self): + return self.fnc + + def process_result(self, future): + self.result = future.result() + + +def dynamic_foo(): + """ + A decorator for dynamically modifying the Foo class to test + CloudpickleProcessPoolExecutor. + + Overrides the `fnc` input of `Foo` with the decorated function. + """ + def as_dynamic_foo(fnc: callable): + return type( + "DynamicFoo", + (Foo,), # Define parentage + { + "__init__": partialmethod( + Foo.__init__, + fnc + ) + }, + ) + + return as_dynamic_foo + +class TestUnpickleableElements(unittest.TestCase): + def test_unpickleable_args(self): + """ + We should be able to use an unpickleable return value -- in this case, a + method of a dynamically defined class. + """ + + @dynamic_foo() + def does_nothing(): + return + + @dynamic_foo() + def slowly_returns_unpickleable(unpickleable_arg): + """ + Returns a complex, dynamically defined variable + """ + sleep(0.1) + unpickleable_arg.result = "input updated" + return unpickleable_arg + + dynamic_dynamic = slowly_returns_unpickleable() + executor = PyMPISingleTaskExecutor() + unpicklable_object = does_nothing() + fs = executor.submit(dynamic_dynamic.run, unpicklable_object) + self.assertEqual(fs.result().result, "input updated") diff --git a/tests/unpicklable_elements/test_callable.py b/tests/unpicklable_elements/test_callable.py new file mode 100644 index 00000000..3bbb2ed8 --- /dev/null +++ b/tests/unpicklable_elements/test_callable.py @@ -0,0 +1,74 @@ +from functools import partialmethod +from time import sleep +import unittest + +from pympipool.mpi.executor import PyMPISingleTaskExecutor + + +class Foo: + """ + A base class to be dynamically modified for testing CloudpickleProcessPoolExecutor. + """ + def __init__(self, fnc: callable): + self.fnc = fnc + self.result = None + + @property + def run(self): + return self.fnc + + def process_result(self, future): + self.result = future.result() + + +def dynamic_foo(): + """ + A decorator for dynamically modifying the Foo class to test + CloudpickleProcessPoolExecutor. + + Overrides the `fnc` input of `Foo` with the decorated function. + """ + def as_dynamic_foo(fnc: callable): + return type( + "DynamicFoo", + (Foo,), # Define parentage + { + "__init__": partialmethod( + Foo.__init__, + fnc + ) + }, + ) + + return as_dynamic_foo + + +class TestUnpickleableElements(unittest.TestCase): + def test_unpickleable_callable(self): + """ + We should be able to use an unpickleable callable -- in this case, a method of + a dynamically defined class. + """ + fortytwo = 42 # No magic numbers; we use it in a couple places so give it a var + + @dynamic_foo() + def slowly_returns_42(): + sleep(0.1) + return fortytwo + + dynamic_42 = slowly_returns_42() # Instantiate the dynamically defined class + self.assertIsInstance( + dynamic_42, + Foo, + msg="Just a sanity check that the test is set up right" + ) + self.assertIsNone( + dynamic_42.result, + msg="Just a sanity check that the test is set up right" + ) + executor = PyMPISingleTaskExecutor() + fs = executor.submit(dynamic_42.run) + fs.add_done_callback(dynamic_42.process_result) + self.assertFalse(fs.done(), msg="Should be running on the executor") + self.assertEqual(fortytwo, fs.result(), msg="Future must complete") + self.assertEqual(fortytwo, dynamic_42.result, msg="Callback must get called") \ No newline at end of file diff --git a/tests/unpicklable_elements/test_exception.py b/tests/unpicklable_elements/test_exception.py new file mode 100644 index 00000000..fe0a33f6 --- /dev/null +++ b/tests/unpicklable_elements/test_exception.py @@ -0,0 +1,55 @@ +from functools import partialmethod +import unittest + +from pympipool.mpi.executor import PyMPISingleTaskExecutor + + +class Foo: + """ + A base class to be dynamically modified for testing CloudpickleProcessPoolExecutor. + """ + def __init__(self, fnc: callable): + self.fnc = fnc + self.result = None + + @property + def run(self): + return self.fnc + + def process_result(self, future): + self.result = future.result() + + +def dynamic_foo(): + """ + A decorator for dynamically modifying the Foo class to test + CloudpickleProcessPoolExecutor. + + Overrides the `fnc` input of `Foo` with the decorated function. + """ + def as_dynamic_foo(fnc: callable): + return type( + "DynamicFoo", + (Foo,), # Define parentage + { + "__init__": partialmethod( + Foo.__init__, + fnc + ) + }, + ) + + return as_dynamic_foo + + +class TestUnpickleableElements(unittest.TestCase): + def test_exception(self): + @dynamic_foo() + def raise_error(): + raise RuntimeError + + re = raise_error() + executor = PyMPISingleTaskExecutor() + fs = executor.submit(re.run) + with self.assertRaises(RuntimeError): + fs.result() diff --git a/tests/unpicklable_elements/test_return.py b/tests/unpicklable_elements/test_return.py new file mode 100644 index 00000000..b54dd066 --- /dev/null +++ b/tests/unpicklable_elements/test_return.py @@ -0,0 +1,74 @@ +from functools import partialmethod +from time import sleep +import unittest + +from pympipool.mpi.executor import PyMPISingleTaskExecutor + + +class Foo: + """ + A base class to be dynamically modified for testing CloudpickleProcessPoolExecutor. + """ + def __init__(self, fnc: callable): + self.fnc = fnc + self.result = None + + @property + def run(self): + return self.fnc + + def process_result(self, future): + self.result = future.result() + + +def dynamic_foo(): + """ + A decorator for dynamically modifying the Foo class to test + CloudpickleProcessPoolExecutor. + + Overrides the `fnc` input of `Foo` with the decorated function. + """ + def as_dynamic_foo(fnc: callable): + return type( + "DynamicFoo", + (Foo,), # Define parentage + { + "__init__": partialmethod( + Foo.__init__, + fnc + ) + }, + ) + + return as_dynamic_foo + + +class TestUnpickleableElements(unittest.TestCase): + def test_unpickleable_return(self): + """ + We should be able to use an unpickleable return value -- in this case, a + method of a dynamically defined class. + """ + + @dynamic_foo() + def does_nothing(): + return + + @dynamic_foo() + def slowly_returns_unpickleable(): + """ + Returns a complex, dynamically defined variable + """ + sleep(0.1) + inside_variable = does_nothing() + inside_variable.result = "it was an inside job!" + return inside_variable + + dynamic_dynamic = slowly_returns_unpickleable() + executor = PyMPISingleTaskExecutor() + fs = executor.submit(dynamic_dynamic.run) + self.assertIsInstance( + fs.result(), + Foo, + ) + self.assertEqual(fs.result().result, "it was an inside job!") \ No newline at end of file diff --git a/tests/unpicklable_elements/test_timeout.py b/tests/unpicklable_elements/test_timeout.py new file mode 100644 index 00000000..3a0c2e29 --- /dev/null +++ b/tests/unpicklable_elements/test_timeout.py @@ -0,0 +1,67 @@ +from functools import partialmethod +from concurrent.futures import TimeoutError +from time import sleep +import unittest + +from pympipool.mpi.executor import PyMPISingleTaskExecutor + + +class Foo: + """ + A base class to be dynamically modified for testing CloudpickleProcessPoolExecutor. + """ + def __init__(self, fnc: callable): + self.fnc = fnc + self.result = None + + @property + def run(self): + return self.fnc + + def process_result(self, future): + self.result = future.result() + + +def dynamic_foo(): + """ + A decorator for dynamically modifying the Foo class to test + CloudpickleProcessPoolExecutor. + + Overrides the `fnc` input of `Foo` with the decorated function. + """ + def as_dynamic_foo(fnc: callable): + return type( + "DynamicFoo", + (Foo,), # Define parentage + { + "__init__": partialmethod( + Foo.__init__, + fnc + ) + }, + ) + + return as_dynamic_foo + + +class TestUnpickleableElements(unittest.TestCase): + def test_timeout(self): + fortytwo = 42 + + @dynamic_foo() + def slow(): + sleep(0.1) + return fortytwo + + f = slow() + executor = PyMPISingleTaskExecutor() + fs = executor.submit(f.run) + self.assertEqual( + fs.result(timeout=30), + fortytwo, + msg="waiting long enough should get the result" + ) + + with self.assertRaises(TimeoutError): + fs = executor.submit(f.run) + fs.result(timeout=0.0001)