Skip to content

Commit

Permalink
worker_db: Only warn on repeated archive read if dataset changed
Browse files Browse the repository at this point in the history 
In larger experiments, it is quite natural for the same dataset
to be read from multiple unrelated components. The only situation
where multiple reads from an archived dataset are problematic is
when the valeu actually changes between reads. Hence, this commit
restricts the warning to the latter situation.
  • Loading branch information
@dnadlinger
dnadlinger committed Jul 11, 2018
1 parent 4843832 commit 4cfb47d
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions artiq/master/worker_db.py
View file
Expand Up @@ -227,9 +227,9 @@ def get(self, key, archive=False):
else:
data = self.ddb.get(key)
if archive:
if key in self.archive:
logger.warning("Dataset '%s' is already in archive, "
"overwriting", key, stack_info=True)
if self.archive.get(key, data) != data:
logger.warning("Older value of dataset '%s' is already in "
"archive, overwriting", key, stack_info=True)
self.archive[key] = data
return data

Expand Down

6 comments on commit 4cfb47d

@jordens
Copy link
Member

Choose a reason for hiding this comment

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

Did you test that, also with arrays?

@dnadlinger
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did (as in, two of our experiments are running with the commit), though not with arrays.

What are you concerned about in particular – did it break any of your code? (Writing AA code in a way that only uses one lookup is an old habit, but probably entirely unnecessary here…)

@jordens
Copy link
Member

Choose a reason for hiding this comment

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

Array equality is element-wise. Even if the shapes are compatible, the resulting boolean array does not automatically become a bool. I am pretty sure this will error when data is an array.

@dnadlinger
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, right, numpy arrays… I have been using Julia a lot lately, and their design for array types is much better – for starters, it has the huge advantage that it doesn't violate the language specification ("Comparisons yield boolean values: True or False").

Easiest fix is probably just to drop the check altogether – opinions? I could also catch ValueErrors to work around the NumPy issue.

@jordens
Copy link
Member

Choose a reason for hiding this comment

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

I'd handle arrays (np.all()) and catch broadcast failure.

@jordens
Copy link
Member

Choose a reason for hiding this comment

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

Or already bail if the naive comparison fails (i.e. don't support multiple retrieval of an array)

Please sign in to comment.