From d6f11b1f47dd4859ddf296ad7e341a85f26574d7 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Fri, 23 Feb 2024 11:53:12 -0800 Subject: [PATCH 1/9] Introduce a flag for _not_ mangling the name of wrapped function jobs --- .../jobs/flex/pythonfunctioncontainer.py | 10 +++- tests/flex/test_pythonfunctioncontainer.py | 46 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/pyiron_base/jobs/flex/pythonfunctioncontainer.py b/pyiron_base/jobs/flex/pythonfunctioncontainer.py index 43cb5c33b..7ce1dbb9b 100644 --- a/pyiron_base/jobs/flex/pythonfunctioncontainer.py +++ b/pyiron_base/jobs/flex/pythonfunctioncontainer.py @@ -44,6 +44,7 @@ def __init__(self, project, job_name): super().__init__(project, job_name) self._function = None self._executor_type = None + self._mangle_name_on_save = True @property def python_function(self): @@ -64,17 +65,24 @@ def __call__(self, *args, **kwargs): def to_hdf(self, hdf=None, group_name=None): super().to_hdf(hdf=hdf, group_name=group_name) self.project_hdf5["function"] = np.void(cloudpickle.dumps(self._function)) + self.project_hdf5["_mangle_name_on_save"] = self._mangle_name_on_save def from_hdf(self, hdf=None, group_name=None): super().from_hdf(hdf=hdf, group_name=group_name) self._function = cloudpickle.loads(self.project_hdf5["function"]) + self._mangle_name_on_save = bool(self.project_hdf5["_mangle_name_on_save"]) def save(self): - job_name = self._function.__name__ + get_hash( + hash_suffix = get_hash( binary=cloudpickle.dumps( {"fn": self._function, "kwargs": self.input.to_builtin()} ) ) + + job_name = self._function.__name__ + if self._mangle_name_on_save: + job_name += hash_suffix + self.job_name = job_name if job_name in self.project.list_nodes(): self.from_hdf() diff --git a/tests/flex/test_pythonfunctioncontainer.py b/tests/flex/test_pythonfunctioncontainer.py index b08662c72..3fe3eb48e 100644 --- a/tests/flex/test_pythonfunctioncontainer.py +++ b/tests/flex/test_pythonfunctioncontainer.py @@ -100,3 +100,49 @@ def test_with_internal_executor(self): job.run() self.assertEqual(job.output["result"], [6, 8, 10, 12]) self.assertTrue(job.status.finished) + + def test_name_mangling(self): + def make_a_simple_job(): + job = self.project.wrap_python_function(my_function) + job.input["a"] = 1 + job.input["b"] = 2 + return job + + job = make_a_simple_job() + self.assertEqual( + job.job_name, + my_function.__name__, + msg="Sanity check" + ) + try: + job.save() + self.assertNotEqual( + job.job_name, + my_function.__name__, + msg="By default, we expect the wrapped job names to get mangled based " + "on their input so multiple calls to the wrap get unique names" + ) + loaded = self.project.load(job.job_name) + self.assertTrue( + loaded._mangle_name_on_save, + msg="The mangling preference should survive saving and loading" + ) + finally: + job.remove() + + job = make_a_simple_job() + job._mangle_name_on_save = False + try: + job.save() + self.assertEqual( + job.job_name, + my_function.__name__, + msg="When requested, the job name should retain its original value" + ) + loaded = self.project.load(job.job_name) + self.assertFalse( + loaded._mangle_name_on_save, + msg="The mangling preference should survive saving and loading" + ) + finally: + job.remove() \ No newline at end of file From 936e911b3fe17924fd5473178e67ac7162ddf370 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Fri, 23 Feb 2024 12:03:02 -0800 Subject: [PATCH 2/9] Don't even override with function name, just keep the given name --- pyiron_base/jobs/flex/pythonfunctioncontainer.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/pyiron_base/jobs/flex/pythonfunctioncontainer.py b/pyiron_base/jobs/flex/pythonfunctioncontainer.py index 7ce1dbb9b..5185eb4ba 100644 --- a/pyiron_base/jobs/flex/pythonfunctioncontainer.py +++ b/pyiron_base/jobs/flex/pythonfunctioncontainer.py @@ -73,18 +73,16 @@ def from_hdf(self, hdf=None, group_name=None): self._mangle_name_on_save = bool(self.project_hdf5["_mangle_name_on_save"]) def save(self): - hash_suffix = get_hash( - binary=cloudpickle.dumps( - {"fn": self._function, "kwargs": self.input.to_builtin()} + if self._mangle_name_on_save: + job_name = self._function.__name__ + get_hash( + binary=cloudpickle.dumps( + {"fn": self._function, "kwargs": self.input.to_builtin()} + ) ) - ) - job_name = self._function.__name__ - if self._mangle_name_on_save: - job_name += hash_suffix + self.job_name = job_name - self.job_name = job_name - if job_name in self.project.list_nodes(): + if self.job_name in self.project.list_nodes(): self.from_hdf() self.status.finished = True else: From fc2572d23b72110eac68249f58d0fda261896f64 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Fri, 23 Feb 2024 12:04:37 -0800 Subject: [PATCH 3/9] Add comment on the private var --- pyiron_base/jobs/flex/pythonfunctioncontainer.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyiron_base/jobs/flex/pythonfunctioncontainer.py b/pyiron_base/jobs/flex/pythonfunctioncontainer.py index 5185eb4ba..00dd58931 100644 --- a/pyiron_base/jobs/flex/pythonfunctioncontainer.py +++ b/pyiron_base/jobs/flex/pythonfunctioncontainer.py @@ -44,7 +44,8 @@ def __init__(self, project, job_name): super().__init__(project, job_name) self._function = None self._executor_type = None - self._mangle_name_on_save = True + self._mangle_name_on_save = True # Automatically rename job using function and + # input values @property def python_function(self): From 77e3ca0a383d256ef087041cb7d8c6c525ca698f Mon Sep 17 00:00:00 2001 From: liamhuber Date: Fri, 23 Feb 2024 12:16:18 -0800 Subject: [PATCH 4/9] Indent and isolate the entire mangling block Then fall back on super() otherwise, to further clarify the intent of the mangling --- pyiron_base/jobs/flex/pythonfunctioncontainer.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pyiron_base/jobs/flex/pythonfunctioncontainer.py b/pyiron_base/jobs/flex/pythonfunctioncontainer.py index 00dd58931..65a356c05 100644 --- a/pyiron_base/jobs/flex/pythonfunctioncontainer.py +++ b/pyiron_base/jobs/flex/pythonfunctioncontainer.py @@ -83,9 +83,11 @@ def save(self): self.job_name = job_name - if self.job_name in self.project.list_nodes(): - self.from_hdf() - self.status.finished = True + if self.job_name in self.project.list_nodes(): + self.from_hdf() + self.status.finished = True + else: + super().save() else: super().save() From 53a23f995ea0324a1da34cad731a3f3f523d1aeb Mon Sep 17 00:00:00 2001 From: Liam Huber Date: Mon, 4 Mar 2024 11:27:42 -0800 Subject: [PATCH 5/9] Update pyiron_base/jobs/flex/pythonfunctioncontainer.py Co-authored-by: Jan Janssen --- pyiron_base/jobs/flex/pythonfunctioncontainer.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pyiron_base/jobs/flex/pythonfunctioncontainer.py b/pyiron_base/jobs/flex/pythonfunctioncontainer.py index 65a356c05..1f4be9c31 100644 --- a/pyiron_base/jobs/flex/pythonfunctioncontainer.py +++ b/pyiron_base/jobs/flex/pythonfunctioncontainer.py @@ -86,10 +86,7 @@ def save(self): if self.job_name in self.project.list_nodes(): self.from_hdf() self.status.finished = True - else: - super().save() - else: - super().save() + super().save() def run_static(self): if ( From f69a3d5703025abad60bb7a798eb7f22853f9628 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Mon, 4 Mar 2024 11:29:53 -0800 Subject: [PATCH 6/9] Fix early return logic That we broke by committing the readability suggestion --- pyiron_base/jobs/flex/pythonfunctioncontainer.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pyiron_base/jobs/flex/pythonfunctioncontainer.py b/pyiron_base/jobs/flex/pythonfunctioncontainer.py index 1f4be9c31..bf6672895 100644 --- a/pyiron_base/jobs/flex/pythonfunctioncontainer.py +++ b/pyiron_base/jobs/flex/pythonfunctioncontainer.py @@ -86,6 +86,7 @@ def save(self): if self.job_name in self.project.list_nodes(): self.from_hdf() self.status.finished = True + return # Without saving super().save() def run_static(self): From 59f19d6adffb9dcaca3a464887abf2580b192977 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Mon, 4 Mar 2024 11:32:11 -0800 Subject: [PATCH 7/9] Use a more descriptive name --- pyiron_base/jobs/flex/pythonfunctioncontainer.py | 13 ++++++++----- tests/flex/test_pythonfunctioncontainer.py | 6 +++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/pyiron_base/jobs/flex/pythonfunctioncontainer.py b/pyiron_base/jobs/flex/pythonfunctioncontainer.py index bf6672895..60f206ccd 100644 --- a/pyiron_base/jobs/flex/pythonfunctioncontainer.py +++ b/pyiron_base/jobs/flex/pythonfunctioncontainer.py @@ -44,8 +44,7 @@ def __init__(self, project, job_name): super().__init__(project, job_name) self._function = None self._executor_type = None - self._mangle_name_on_save = True # Automatically rename job using function and - # input values + self._automatically_rename_on_save_using_input = True @property def python_function(self): @@ -66,15 +65,19 @@ def __call__(self, *args, **kwargs): def to_hdf(self, hdf=None, group_name=None): super().to_hdf(hdf=hdf, group_name=group_name) self.project_hdf5["function"] = np.void(cloudpickle.dumps(self._function)) - self.project_hdf5["_mangle_name_on_save"] = self._mangle_name_on_save + self.project_hdf5[ + "_automatically_rename_on_save_using_input" + ] = self._automatically_rename_on_save_using_input def from_hdf(self, hdf=None, group_name=None): super().from_hdf(hdf=hdf, group_name=group_name) self._function = cloudpickle.loads(self.project_hdf5["function"]) - self._mangle_name_on_save = bool(self.project_hdf5["_mangle_name_on_save"]) + self._automatically_rename_on_save_using_input = bool( + self.project_hdf5["_automatically_rename_on_save_using_input"] + ) def save(self): - if self._mangle_name_on_save: + if self._automatically_rename_on_save_using_input: job_name = self._function.__name__ + get_hash( binary=cloudpickle.dumps( {"fn": self._function, "kwargs": self.input.to_builtin()} diff --git a/tests/flex/test_pythonfunctioncontainer.py b/tests/flex/test_pythonfunctioncontainer.py index 3fe3eb48e..23d144a23 100644 --- a/tests/flex/test_pythonfunctioncontainer.py +++ b/tests/flex/test_pythonfunctioncontainer.py @@ -124,14 +124,14 @@ def make_a_simple_job(): ) loaded = self.project.load(job.job_name) self.assertTrue( - loaded._mangle_name_on_save, + loaded._automatically_rename_on_save_using_input, msg="The mangling preference should survive saving and loading" ) finally: job.remove() job = make_a_simple_job() - job._mangle_name_on_save = False + job._automatically_rename_on_save_using_input = False try: job.save() self.assertEqual( @@ -141,7 +141,7 @@ def make_a_simple_job(): ) loaded = self.project.load(job.job_name) self.assertFalse( - loaded._mangle_name_on_save, + loaded._automatically_rename_on_save_using_input, msg="The mangling preference should survive saving and loading" ) finally: From 171e848359fdc063a0d6aca2dd4f6cf08861f6c9 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Mon, 4 Mar 2024 11:33:07 -0800 Subject: [PATCH 8/9] fix formatting --- tests/flex/test_pythonfunctioncontainer.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/flex/test_pythonfunctioncontainer.py b/tests/flex/test_pythonfunctioncontainer.py index 23d144a23..595498a8c 100644 --- a/tests/flex/test_pythonfunctioncontainer.py +++ b/tests/flex/test_pythonfunctioncontainer.py @@ -55,7 +55,9 @@ def test_with_executor(self): self.assertIsNone(job.server.future.result()) self.assertTrue(job.server.future.done()) - @unittest.skipIf(os.name == "nt", "Starting subprocesses on windows take a long time.") + @unittest.skipIf( + os.name == "nt", "Starting subprocesses on windows take a long time." + ) def test_terminate_job(self): job = self.project.wrap_python_function(my_sleep_funct) job.input["a"] = 5 @@ -145,4 +147,4 @@ def make_a_simple_job(): msg="The mangling preference should survive saving and loading" ) finally: - job.remove() \ No newline at end of file + job.remove() From 84e308916c0ca825511996cd4b970c3705330945 Mon Sep 17 00:00:00 2001 From: pyiron-runner Date: Mon, 4 Mar 2024 19:37:14 +0000 Subject: [PATCH 9/9] Format black --- pyiron_base/jobs/flex/pythonfunctioncontainer.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pyiron_base/jobs/flex/pythonfunctioncontainer.py b/pyiron_base/jobs/flex/pythonfunctioncontainer.py index 60f206ccd..01ad1c6aa 100644 --- a/pyiron_base/jobs/flex/pythonfunctioncontainer.py +++ b/pyiron_base/jobs/flex/pythonfunctioncontainer.py @@ -65,9 +65,9 @@ def __call__(self, *args, **kwargs): def to_hdf(self, hdf=None, group_name=None): super().to_hdf(hdf=hdf, group_name=group_name) self.project_hdf5["function"] = np.void(cloudpickle.dumps(self._function)) - self.project_hdf5[ - "_automatically_rename_on_save_using_input" - ] = self._automatically_rename_on_save_using_input + self.project_hdf5["_automatically_rename_on_save_using_input"] = ( + self._automatically_rename_on_save_using_input + ) def from_hdf(self, hdf=None, group_name=None): super().from_hdf(hdf=hdf, group_name=group_name)