Skip to content

Commit

Permalink
[minor] Opt in hash for pythonfunction (#1358)
Browse files Browse the repository at this point in the history
* Introduce a flag for _not_ mangling the name of wrapped function jobs

* Don't even override with function name, just keep the given name

* Add comment on the private var

* Indent and isolate the entire mangling block

Then fall back on super() otherwise, to further clarify the intent of the mangling

* Change the mangling default to false on the job class

* Maintain (now optional) mangling behaviour on the wrapper

* Update test

* Refactor: rename private var

* Clarify intent to future devs

* Allow a name to be passed to the function wrapper job

* Update test

* Rename test

* Update pyiron_base/jobs/flex/pythonfunctioncontainer.py

Co-authored-by: Jan Janssen <[email protected]>

* Fix early return logic

That we broke by committing the readability suggestion

* Use a more descriptive name

* fix formatting

* Format black

* 🐛 finish renaming attribute

* Format black

* Cover the found-save branch

* Never re-save existing jobs

---------

Co-authored-by: Jan Janssen <[email protected]>
Co-authored-by: pyiron-runner <[email protected]>
  • Loading branch information
3 people authored Mar 7, 2024
1 parent 3b56972 commit 8d187c8
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 45 deletions.
19 changes: 11 additions & 8 deletions pyiron_base/jobs/flex/pythonfunctioncontainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,12 @@ def __init__(self, project, job_name):
super().__init__(project, job_name)
self._function = None
self._executor_type = None
self._automatically_rename_on_save_using_input = True
self._automatically_rename_on_save_using_input = False
# Automatically rename job using function and input values at save time
# This is useful for the edge case where these jobs are created from a wrapper
# and automatically assigned a name based on the function name, but multiple
# jobs are created from the same function (and thus distinguished only by their
# input)

@property
def python_function(self):
Expand Down Expand Up @@ -78,18 +83,16 @@ def from_hdf(self, hdf=None, group_name=None):

def save(self):
if self._automatically_rename_on_save_using_input:
job_name = self._function.__name__ + get_hash(
self.job_name = self.job_name + get_hash(
binary=cloudpickle.dumps(
{"fn": self._function, "kwargs": self.input.to_builtin()}
)
)

self.job_name = job_name

if self.job_name in self.project.list_nodes():
self.from_hdf()
self.status.finished = True
return # Without saving
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):
Expand Down
12 changes: 10 additions & 2 deletions pyiron_base/project/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,12 +412,19 @@ def create_table(self, job_name="table", delete_existing_job=False):
table.analysis_project = self
return table

def wrap_python_function(self, python_function):
def wrap_python_function(
self, python_function, job_name=None, automatically_rename=True
):
"""
Create a pyiron job object from any python function
Args:
python_function (callable): python function to create a job object from
job_name (str | None): The name for the created job. (Default is None, use
the name of the function.)
automatically_rename (bool): Whether to automatically rename the job at
save-time to append a string based on the input values. (Default is
True.)
Returns:
pyiron_base.jobs.flex.pythonfunctioncontainer.PythonFunctionContainerJob: pyiron job object
Expand All @@ -440,8 +447,9 @@ def wrap_python_function(self, python_function):
"""
job = self.create.job.PythonFunctionContainerJob(
job_name=python_function.__name__
job_name=python_function.__name__ if job_name is None else job_name
)
job._automatically_rename_on_save_using_input = automatically_rename
job.python_function = python_function
return job

Expand Down
101 changes: 66 additions & 35 deletions tests/flex/test_pythonfunctioncontainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,48 +103,79 @@ def test_with_internal_executor(self):
self.assertEqual(job.output["result"], [6, 8, 10, 12])
self.assertTrue(job.status.finished)

def test_name_mangling(self):
def make_a_simple_job():
def test_name_options(self):
with self.subTest("Auto name and rename"):
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,

self.assertEqual(
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._automatically_rename_on_save_using_input,
msg="The mangling preference should survive saving and loading"
job.job_name,
msg="Docs claim job name takes function name by default"
)
finally:
job.remove()
pre_save_name = job.job_name
try:
job.save()
self.assertNotEqual(
pre_save_name,
job.job_name,
msg="Docs claim default is to modify the name on save"
)
self.assertTrue(
pre_save_name in job.job_name,
msg="The job name should still be based off the original name"
)
finally:
job.remove()

with self.subTest("Custom name and rename"):
name = "foo"
job = self.project.wrap_python_function(my_function, job_name=name)
job.input["a"] = 1
job.input["b"] = 2

job = make_a_simple_job()
job._automatically_rename_on_save_using_input = False
try:
job.save()
self.assertEqual(
name,
job.job_name,
my_function.__name__,
msg="When requested, the job name should retain its original value"
msg="Provided name should be used"
)
loaded = self.project.load(job.job_name)
self.assertFalse(
loaded._automatically_rename_on_save_using_input,
msg="The mangling preference should survive saving and loading"
try:
job.save()
self.assertNotEqual(
name,
job.job_name,
msg="Docs claim default is to modify the name on save"
)
print("NAME STUFF", name, job.job_name)
self.assertTrue(
name in job.job_name,
msg="The job name should still be based off the original name"
)
finally:
job.remove()

with self.subTest("No rename"):
job = self.project.wrap_python_function(
my_function, automatically_rename=False
)
finally:
job.remove()
job.input["a"] = 1
job.input["b"] = 2

pre_save_name = job.job_name
try:
job.save()
self.assertEqual(
pre_save_name,
job.job_name,
msg="We should be able to deactivate the automatic renaming"
)
n_ids = len(self.project.job_table())
job.save()
self.assertEqual(
n_ids,
len(self.project.job_table()),
msg="When re-saving, the job should be found and loaded instead"
)
finally:
job.remove()

0 comments on commit 8d187c8

Please sign in to comment.