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

Defer statepoint instantiation, unify reset_statepoint logic #497

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 23 additions & 10 deletions signac/contrib/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,6 @@ def __init__(self, project, statepoint=None, _id=None):
else:
# Only an id was provided. State point will be loaded lazily.
self._id = _id
self._statepoint = _StatePointDict(
jobs=[self], filename=self._statepoint_filename
)
self._statepoint_requires_init = True

def _initialize_lazy_properties(self):
Expand Down Expand Up @@ -383,7 +380,17 @@ def reset_statepoint(self, new_statepoint):
The job's new state point.

"""
self._statepoint.reset(new_statepoint)
if self._statepoint_requires_init:
# Instantiate state point data lazily - no load is required, since
# we are provided with the new state point data.
self._statepoint = _StatePointDict(
vyasr marked this conversation as resolved.
Show resolved Hide resolved
jobs=[self], filename=self._statepoint_filename
)
self._statepoint_requires_init = False
self.statepoint.reset(new_statepoint)

# Update the project's state point cache when loaded lazily
self._project._register(self.id, new_statepoint)

def update_statepoint(self, update, overwrite=False):
"""Change the state point of this job while preserving job data.
Expand Down Expand Up @@ -427,9 +434,12 @@ def update_statepoint(self, update, overwrite=False):
if not overwrite:
for key, value in update.items():
if statepoint.get(key, value) != value:
raise KeyError(key)
raise KeyError(
f"Key {key} was provided but already exists in the "
"mapping with another value."
)
statepoint.update(update)
self._statepoint.reset(statepoint)
self.reset_statepoint(statepoint)

@property
def statepoint(self):
Expand All @@ -456,6 +466,9 @@ def statepoint(self):
"""
if self._statepoint_requires_init:
# Load state point data lazily (on access).
self._statepoint = _StatePointDict(
jobs=[self], filename=self._statepoint_filename
)
statepoint = self._statepoint.load(self.id)

# Update the project's state point cache when loaded lazily
Expand All @@ -474,7 +487,7 @@ def statepoint(self, new_statepoint):
The new state point to be assigned.

"""
self._statepoint.reset(new_statepoint)
self.reset_statepoint(new_statepoint)

@property
def sp(self):
Expand Down Expand Up @@ -657,7 +670,7 @@ def init(self, force=False):
try:
# Attempt early exit if the state point file exists and is valid.
try:
statepoint = self._statepoint.load(self.id)
statepoint = self.statepoint.load(self.id)
except Exception:
# Any exception means this method cannot exit early.

Expand All @@ -674,8 +687,8 @@ def init(self, force=False):
# The state point save will not overwrite an existing file on
# disk unless force is True, so the subsequent load will catch
# when a preexisting invalid file was present.
self._statepoint.save(force=force)
statepoint = self._statepoint.load(self.id)
self.statepoint.save(force=force)
statepoint = self.statepoint.load(self.id)

# Update the project's state point cache if the saved file is valid.
self._project._register(self.id, statepoint)
Expand Down
33 changes: 33 additions & 0 deletions tests/test_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,39 @@ def test_reset_statepoint_job(self):
with pytest.raises(DestinationExistsError):
src_job.reset_statepoint(dst)

def test_reset_statepoint_job_lazy_access(self):
bdice marked this conversation as resolved.
Show resolved Hide resolved
key = "move_job"
d = testdata()
src = test_token
dst = dict(test_token)
dst["dst"] = True
src_job = self.open_job(src)
src_job.document[key] = d
assert key in src_job.document
assert len(src_job.document) == 1
src_job.data[key] = d
assert key in src_job.data
assert len(src_job.data) == 1
# Clear the project's state point cache to force lazy load
self.project._sp_cache.clear()
src_job_by_id = self.open_job(id=src_job.id)
# Check that the state point will be instantiated lazily during the
# call to reset_statepoint
assert src_job_by_id._statepoint_requires_init
src_job_by_id.reset_statepoint(dst)
src_job = self.open_job(src)
dst_job = self.open_job(dst)
assert key in dst_job.document
assert len(dst_job.document) == 1
assert key not in src_job.document
assert key in dst_job.data
assert len(dst_job.data) == 1
assert key not in src_job.data
with pytest.raises(RuntimeError):
src_job.reset_statepoint(dst)
with pytest.raises(DestinationExistsError):
src_job.reset_statepoint(dst)

@pytest.mark.skipif(not H5PY, reason="test requires the h5py package")
def test_reset_statepoint_project(self):
key = "move_job"
Expand Down