Add mechanism for re-loading the statistic files in live #4816
Conversation
pycbc/events/stat.py
Outdated
| def files_changed(self): | ||
| """ | ||
| Compare hashes of files now with the ones we have cached | ||
| """ | ||
| changed_file_hashes = self.get_file_hashes() | ||
| for stat, old_hash in self.file_hashes.items(): | ||
| if changed_file_hashes[stat] != old_hash: | ||
| logger.info( | ||
| "%s statistic file %s has changed", | ||
| ''.join(self.ifos), | ||
| stat, | ||
| ) | ||
| else: | ||
| logger.debug( | ||
| "No %s statistic files have changed", | ||
| ''.join(self.ifos) | ||
| ) | ||
| return [] | ||
|
|
||
| return list(changed_file_hashes.keys()) |
There was a problem hiding this comment.
This code does not seem right to me.
My understanding of the
for:
...
else:
code is that the else block will always be executed unless the loop ends by a break command. So this seems like it will always return an empty list?
Additionally, if it somehow avoids executing the else block, it seems like the returned list will always be the full set of files, and not just the files that were actually changed?
Everything else in this MR makes sense to me.
There was a problem hiding this comment.
Thanks for pointing that out - for some reason I had it in my head that if you had if statements which were never met, then the else would be implemented. Kind of backwards to what it actually is I suppose.
I have updated to correct functionality with the latest commit
|
I think the failing tests are in a dependency, I'll ask in the code slack channel |
|
@GarethCabournDavies Looks like all of the required checks have now passed. Are you ready to merge? |
|
Yup, I just hadn't noticed that the latest reattempt had worked! |
* Add mechanism for re-loading the statistic files * CC fixes * CC 2 * Fix for/else statement - wrongly implemented
Statistic files will be changed by a supervisor script in live for future uses. This PR adds the ability to check the sha1 hashes of the file and compare it to a stored copy so that if the file has changed, it can be reloaded.
In this usage, the file in the command line arguments would be overwritten (or a symlink which is updated), and then the statistic would refresh at an appropriate rate.
Standard information about the request
This is a new feature
This change affects the live search; Offline code is touched but not affected
This change changes scientific output
This change follows style guidelines (See e.g. PEP8), has been proposed using the contribution guidelines
Motivation
We want to be able to use a proper ranking statistic in Live, but do not want to have to use static files, or restart the search regularly in order to update them. This change means that the ranking statistic files can be reloaded
Contents
Links to any issues or associated PRs
Link to draft PR: https://github.com/GarethCabournDavies/pycbc/pull/5/files
#4689, #4527, #4813
Note that this will significantly clash with #4608
Testing performed
Started pycbc live - printed some values from the fits files.
Updated fits files, checked that the values are updated after this, and that the updated files were logged
The author of this pull request confirms they will adhere to the code of conduct