PSB-242: add trace extraction roi exclusion labels

[mikejhuang]

Trace extraction exclusion labels

1. "empty_roi_mask"
2. "empty_neuropil_mask"

Summary of changes

1. Add exclusion label attributes to 'OphysROI'
2. Save exclusion labels to OphysROI objects in db after trace_extraction is complete
3. Run step 2 as an additional_insert in the save_job_run_to_db for the trace_extraction task
4. unit tests

[aamster]

@mikejhuang can you please describe why these flags need to be added?

[mikejhuang]

@mikejhuang can you please describe why these flags need to be added?

Any flags detected in trace extraction will lead to that trace being invalidated and stored as nan values. Subsequent steps in the pipeline will use the flags to mask the invalidated traces before processing.

Thus, it's important to add these flags so that subsequent steps (demix, neuropil correction) can load these flags and mask the invalidated traces.

[mikejhuang]

The code looks good, but I believe we don't need to add flags for these things and should instead filter out these rois as invalid. I also believe these flags should be calculated in the segmentation step, not trace extraction since it has to do with segmentation and not trace extraction.

I think it makes sense to filter out roi's that are empty in segmentation(not sure why they would be and why there was the logic placed in trace extraction to check for it).

However, I don't think it's a good idea to remove the roi's with an empty neuropil mask. For one, it's important for reproducibility since the ROIs with empty neuropil masks are still used to generate other neuropil masks since ROIs overlapping the neuropil masks are subtracted from it. Secondly, the scientists might be interested in that information and we shouldn't decide this isn't necessary for them.

We can implement the logic to generate the neuropil mask in segment_postprocess and I considered doing it there alongside where the is_in_motion_border check is done, but that would also require us to store the neuropil masks at the segmentation step and potentially store that in the database alongside ROI masks. That's a major change. For a nice to have, I didn't think it was worth it.

[aamster]

@mikejhuang so is the plan to implement the logic to filter out the 0 area rois and remove the "empty_roi_mask" flag, and then create a separate ticket to implement neuropil mask calculation, flagging and storage in the segmentation step? If we were to take on that ticket then we would have to undo some of the changes in this PR which is extra work.

[mikejhuang]

@mikejhuang so is the plan to implement the logic to filter out the 0 area rois and remove the "empty_roi_mask" flag, and then create a separate ticket to implement neuropil mask calculation, flagging and storage in the segmentation step? If we were to take on that ticket then we would have to undo some of the changes in this PR which is extra work.

I think removing the empty ROIs in segmentation makes sense and I can add that to this PR and remove the empty_roi flag.

Should we decide here or in the backlog refinement whether it's worth the major changes with the neuropil_mask for the nice to haves?

[aamster]

@mikejhuang could you make a ticket for it and then we can discuss at backlog refinement. I honestly don't think it's too involved.

[mikejhuang]

It's determined that logic already exists to remove empty ROI masks after the morphological transform operation. Thus we should ensure unit testing can confirm that no empty ROI can get outputted from SegmentPostprocess