Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subsampling nwb #78

Draft
wants to merge 10 commits into
base: variable-rois
Choose a base branch
from
Draft

Subsampling nwb #78

wants to merge 10 commits into from

Conversation

alessandrofelder
Copy link
Collaborator

No description provided.

Copy link
Contributor

@ageorgou ageorgou left a comment

Choose a reason for hiding this comment

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

Looks good, apart from the TDMS reading order (see comment).

Otherwise, find_used_planes may also need to be updated as I think it always return an empty list right now. This doesn't matter much because its output is only used in copy_pockels_file (where it's ignored anyway, see below) and copy_zstack (and I think it's fine to not include those images?)

Unrelated to these changes, but it looks like the copy_pockels_file is copying the whole file (which may be what we need, if we want to retain all the Zstack "locations", but in that case could be simplified a bit)

Comment on lines +191 to +192
n_all_trials = len(nwb['/intervals/trials/id'])
return np.int(nwb['acquisition/ROI_001_Red/timestamps'].shape[0] / n_all_trials)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we get this directly from nwb['general/silverlab_optophysiology/cycles_per_trial']?

Comment on lines 216 to 221
for roi_index, roi_to_keep in enumerate(all_rois[0:nrois]):
starts = np.arange(cycles_per_trial(nwb)) * total_pixels + previous_pixels[roi_index]
stops = starts + all_roi_pixels[roi_index]
inds = np.array([np.arange(start, stop) for (start, stop) in zip(starts, stops)])
ch_data = ch_obj.data[inds].flatten()
subset = np.concatenate((subset, ch_data))
Copy link
Contributor

Choose a reason for hiding this comment

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

So this gets all the data for one ROI across all cycles, then appends the data for the next ROI for all cycles etc, but the original TDMS is structured the other way around: it has data for all ROIs for the first cycle, then all ROIs for the second cycle etc. This may be why we the sampled file is not consistent with the original!

May be wrong!

Previous version had all data from the first ROI
followed by the second etc, but the original TDMS stores data
in complete cycles (first cycle for all ROIs, then second cycle
for all ROIs etc).

This is a bit less general than the previous version,
which could have been easily adapted for non-consecutive ROIs.
However, the whole file is based on the assumption that we are
keeping only the first N ROIs, so the simplification is probably
worth the loss of generality in one function...
@ageorgou
Copy link
Contributor

ageorgou commented Dec 13, 2020

I've pushed some changes which I think should read things in the right order, but I haven't tried them! (the second commit should do the same, but removing some loops)

this ensures the dimensions listed ROI.dat and those of the TDMS data are
consistent with each other.
e.g. so we don't have to start from ROI 1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants