Add symlink to Processing for ZDriftMetrics
#5
Conversation
| @@ -353,6 +351,7 @@ class ZDriftMetrics(dj.Computed): | |||
| -> scan.Scan | |||
| -> ZDriftParamSet | |||
| --- | |||
| exceeds_threshold: boolean # `True` if any value in z_drift > threshold from drift_params. | |||
There was a problem hiding this comment.
Instead of a boolean like this, why not directly store a longblob of the frame indices of the frames exceeding the threshold (call it bad_frames)?
There was a problem hiding this comment.
Yes, that's a good idea. It can be fetched directly in the make() function. Does the symlink part look correct? I was following the example of the ephys implementation of it.
kushalbakshi
changed the title
ZDriftMetricsProcessing for ZDriftMetrics
|
@ttngu207, this is ready for review. This version of This indicates |
| @@ -522,16 +524,33 @@ def make(self, key): | |||
| else: | |||
| raise NotImplementedError("Unknown method: {}".format(method)) | |||
| elif task_mode == "trigger": | |||
| drop_frames = (ZDriftMetrics & key).fetch1("bad_frames") | |||
There was a problem hiding this comment.
Because there may be multiple entries in ZDriftMetrics for one particular scan (corresponding to different zdrift_paramset), there is a risk with fetch1() here.
There is no guarantee that there is only one. If there are multiple ZDriftMetrics for this scan, which zdrift_param are we going with? Can we make the assumption to go with the first one?
There was a problem hiding this comment.
That is a good point. We can probably assume the first one for now, unless we want to refactor the module to allow zdrift_paramset_idx as an additional primary key for all downstream tables from ZDriftMetrics
There was a problem hiding this comment.
Since this is a fork separate from the main element, I'm open to either approach. Let me know which one you prefer
There was a problem hiding this comment.
@kushalbakshi , let's go with the assumption that there is only one ZDriftMetrics for the particular scan, raise an error with a meaningful message if there are more than one.
| drop_frames = (ZDriftMetrics & key).fetch("bad_frames") | ||
| if len(drop_frames) > 1: |
There was a problem hiding this comment.
You can simplify by doing try-catch around the fetch1() here
This implementation is one suggestion but it might make more sense to create a table downstream from
ZDriftMetricscalledDropFrameswith a foreign key reference toZDriftMetrics.DropFrameswould also have an additional boolean attribute indicating whether it was necessary to drop frames or not based on the drift values generated for each frame inZDriftMetrics. IfFalse, theProcessingtable will look for the raw data in the inbox, ifTruetheProcessingtable will look for a symlink in the outbox.