Add module for aligning local timestamps to the Harp clock #24
Conversation
harp/clock.py
Outdated
| diffs = np.abs(np.diff(harp_times)) | ||
|
|
||
| # Find the indices of the outliers | ||
| outliers = np.where(diffs > 1)[0] |
There was a problem hiding this comment.
Isn't this to strict? I guess it's possible that the harp period is slightly over 1s
One possibility could be adding a delta_eps=0.01 argument and use np.where(diffs > 1 + delta_eps)[0]
There was a problem hiding this comment.
The Harp times will always be exactly 1 s apart, this is programmed into the firmware.
Right now this approach is not robust to the Harp clock resetting, or gaps in the ephys recording. A better solution might be to look for any Harp times with outliers both before and after, and remove these entirely. That will make it possible to detect jumps in the Harp timestamps that are actually valid.
harp/clock.py
Outdated
| if len(outliers) == 1: | ||
| warnings.warn("One outlier found in the decoded Harp clock. Correcting...") | ||
| elif len(outliers) > 1: | ||
| warnings.warn( | ||
| f"{len(outliers)} outliers found in the decoded Harp clock. Correcting..." | ||
| ) |
There was a problem hiding this comment.
Why not just
| if len(outliers) == 1: | |
| warnings.warn("One outlier found in the decoded Harp clock. Correcting...") | |
| elif len(outliers) > 1: | |
| warnings.warn( | |
| f"{len(outliers)} outliers found in the decoded Harp clock. Correcting..." | |
| ) | |
| if len(outliers) > 0: | |
| warnings.warn( | |
| f"{len(outliers)} outliers found in the decoded Harp clock. Correcting..." | |
| ) |
There was a problem hiding this comment.
This was done to ensure the pluralization of "outliers" in the warning message was correct. It could also be done with a if statement inside the fstring.
harp/clock.py
Outdated
| Harp clock times in seconds | ||
| """ | ||
|
|
||
| min_delta = sample_rate / 2 # 0.5 seconds |
There was a problem hiding this comment.
| min_delta = sample_rate / 2 # 0.5 seconds | |
| min_delta = 1 / sample_rate / 2 # 0.5 seconds |
min_delta is in s
There was a problem hiding this comment.
If we assume the timestamps are in seconds, this is not needed
harp/clock.py
Outdated
|
|
||
| min_delta = sample_rate / 2 # 0.5 seconds | ||
|
|
||
| barcode_edges = get_barcode_edges(timestamps_or_sample_numbers, min_delta) |
There was a problem hiding this comment.
Since min_delta is half of the period in s, I would suggest forcing the use of timestamps instead of timestamps_or_sample_numbers. What do you think?
There was a problem hiding this comment.
Yeah, I think just enforcing the use of timestamps in seconds will create less opportunities for user errors.
There was a problem hiding this comment.
This is fixed in the latest set of commits
|
@jsiegle this looks great, thanks! Can you run |
|
I ran it and it didn't change anything -- what does it look like it missed? |
When acquiring data from a device that is not synchronized to the global Harp clock, Harp timestamps can be encoded as barcodes that are acquired by one of the device's digital input lines. This allows post-hoc synchronization with the Harp clock.
This PR adds a module that can decode the Harp times (in seconds) from set of digital line transitions acquired by a non-Harp device, and use this info to align any of that device's timestamps to the global Harp clock.
This is not directly related to reading data saved by Harp devices, but it's utility that will be needed for a wide range of experiments involving Harp. I thought this repo was the best place for this code to live, but let me know if it belongs somewhere else.