Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[patch] Introduce a flag for _not_ mangling the name of wrapped function jobs #1356

Merged
merged 11 commits into from
Mar 7, 2024
25 changes: 17 additions & 8 deletions pyiron_base/jobs/flex/pythonfunctioncontainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +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 # Automatically rename job using function and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self._mangle_name_on_save = True # Automatically rename job using function and
self._overwrite_job_name_by_hash = True

I feel when we have to explain the variable name in a comment, then maybe the variable name is not intuitive.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was aware of this deficiency -- I fixed it in the stacked PR, but it was still unprofessional of me to just leave it knowingly-bad here.

I find your suggestion loses information on when/why we would rename this, and focuses on the implementation (hashing) instead of the functionality (leveraging input values), so I opted to go with _automatically_rename_on_save_using_input.

This is definitely an improvement, although in the stacked PR I'm still inclined to leave to comment to give even more depth to why we're doing this.

# input values

@property
def python_function(self):
Expand All @@ -64,21 +66,28 @@ 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
jan-janssen marked this conversation as resolved.
Show resolved Hide resolved

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(
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()}
)
)
)
self.job_name = job_name
if job_name in self.project.list_nodes():
self.from_hdf()
self.status.finished = True

self.job_name = job_name

if self.job_name in self.project.list_nodes():
self.from_hdf()
self.status.finished = True
else:
super().save()
else:
super().save()
liamhuber marked this conversation as resolved.
Show resolved Hide resolved

Expand Down
46 changes: 46 additions & 0 deletions tests/flex/test_pythonfunctioncontainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Loading