-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add multi-run workflow synchronized with BIDS dataset #219
Conversation
From some documentation I found from Siemens on image reconstruction:
From an answer given by Chris Roden (dcm2niix):
So, if the reconstruction involves something more than a straight FFT (i.e., GRAPPA, SENSE, etc.), that will explain the ~20 sec delay you find between the If I remember correctly, for Siemens scanners, the "scanner trigger" is sent at the time of the excitation pulse of the first slice of each (collected) volume. So the very first trigger will be sent just a few milliseconds before the first chunk of data for the first slice for the first volume is acquired ( Regarding the time resolution (seconds vs. sub-second): I think the purpose of |
Thanks @pvelasco! I'm glad to know that AcquisitionTime is the right one, although it looks like heudiconv grabs acq_time from ContentTime instead of AcquisitionTime. It might be best to use AcquisitionTime instead, although I can follow up about that in nipy/heudiconv#447.
Yes, the scan times are used to find the trigger periods that correspond to each functional file, but this approach won't work for single-scan physio files, because it relies on the exact times between physio trigger periods as well as those between scan onsets. At the moment, my code finds the trigger signal that would have the shortest delay from the scan acquisition time, rather than the first trigger signal. My logic is that, since the triggers occur at variable delays, the one with the shortest delay is the one that is closest to the actual start of acquisition, and is thus the one we should build our timing information from. To find that time, we need scan times that are as accurate as possible. Well, actually we need relative onsets to be accurate. The absolute scan times are not used, because I didn't want to assume that the physio and scan computers' clocks were perfectly synchronized, and because the BioPac segment timestamps seem to only have second-level resolution. |
Will need to drop before general use, but there is a plan in place. Just not sure how to implement it yet.
The functions are now in a state that I can call them sequentially in a notebook (see here). I have also made some time series figures of the physio and functional timing from the results (see here for the notebook and below for the figures). You might notice a few odd things below:
|
phys2bids/conversion.py
Outdated
from operator import itemgetter | ||
from itertools import groupby | ||
physio_data = bioread.read_file(physio_file) | ||
trigger_channel = physio_data.channels[-1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: would you consider making your code more generic by allowing more flexibility in the selection of the trigger_channel
?
Something like:
trigger_idx = [c.name.lower() for c in physio_data.channels].index('trigger')
trigger_channel = physio_data.channels[trigger_idx]
or:
trigger_channel = [c for c in physio_data.channels if c.name.lower()=='trigger'][0]
(or maybe just 'trig' in c.name.lower()
, in case people use abbreviations...)
Then, you could use the last channel as a fallback option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One of the things on my to-do list is to use the native phys2bids
classes. I don't know much about them at the moment, so that may take some time, but I believe that selecting the trigger channel is one of the possible parameters for the main physio container object.
That said, if your interest is more in making it easier to test/debug for the near future, I'm happy to make the trigger channel selection more flexible.
Hi all, I have read through what people have written but I have done so rather quickly (so as to catch up), so sorry if I misunderstand things. In general, I agree with @smoia - I think I will need to see this code in action to understand it properly. I also lean towards have an “old triggers” column and a new “corrected” triggers column, as it probably helps for troubleshooting. In general, this PR looks like its adding some great functionality, but it may only be relevant for certain experimental and equipment set-ups? I will explain a bit about my experimental set-up, below, and you can let me know if you agree! Rationale/Concept - I don’t think I can remember an occasion where I have had just a few missing triggers in my physiological recordings, however I have had occasions where I have not recorded any trigger information at all, alongside the physiological data (due to forgetting to plug the trigger connection in ... ) which is very annoying. From what I have picked up, I think I have quite a different set-up to you @tsalo. I don’t have any synchronization between task (stimuli presentation) and trigger recording alongside the physiological data. I just record all the volume triggers that the scanner sends during fMRI sequences, but this does not trigger anything else to happen. (Sometimes I get the the stimuli presentation to be triggered by the first volume trigger of a scan, but that is separate from the physiological recording equipment). Implementation - I am struggling to work out how this would exactly apply for the type of data I collect, so I will explain what we do. We first turn on our physiological recording software a while before we start any scanning, as it takes time to attach all the peripheral devices to a person, and check the signals look reasonable. We then record continuously throughout the whole MRI session i.e. all physiological data from one MRI session is contained within one file. Within this recording of physiological data, we record all MRI volume triggers. Therefore the start time of the physiological recording file is not synched at all with any other start time (task, MRI scan). However, I understand that this code is going to look at the relative timings between MRI scans to work out the relative physiological timing blocks? That makes some sense to me, but does it therefore always assume you have the first trigger of an fMRI scan, to get the onset of this block correct? And then if there are any triggers missing in the middle it can fill them in just fine? Otherwise, I am struggling to work out how this works without having a trigger recorded at the onset of an fMRI scan. |
The ability of the workflow to correct trigger time series is only relevant for some setups, and the workflow will most definitely not work with single-run setups, but the general ability of the workflow to identify and label physio runs associated with specific imaging runs is broadly useful. Basically, you shouldn't have to feed in any information beyond a path to the BIDS dataset and the subject/session to get your physio data.
Your trigger time series is going to be accurate (unlike mine), but it's also completely unlabeled. You still need to use a heuristic or something to identify which block of physio data corresponds to which imaging file in the BIDS dataset. This should automate that.
You need starts of physio trigger periods to generally match up with starts of fMRI scans for this to work, but a lot of the extra stuff I put into the workflow is to make sure it will work in cases where the first scan trigger or imaging data are missing. Think of it like this. You have two arrays of "onsets" with the same scale (seconds) but different start points. You know the durations and "names" for the different onsets for one of the arrays, but not the other. Here's some pseudocode to illustrate: physio_onsets = [1.5, 7, 12.2, 22, 27] # from physio file
scan_onsets = [0.8, 6.3, 10.7, 20.5, 25.5] # from BIDS dataset
scan_durations = [1, 2, 3, 4, 5] # from BIDS dataset
scan_names = ['a', 'b', 'c', 'd', 'e'] # from BIDS dataset
# the times between the onsets at the same
delta_physio = np.diff(physio_onsets) # [5.5., 5.2, 9.8, 5]
delta_scans = np.diff(scan_onsets) # [5.5., 5.2, 9.8, 5]
delta_physio == delta_scans # True
# So now we find what the scan onsets (and durations) would be
# in terms of the physio signal.
# I'll estimate the onset from just the first scan and physio block
# because we *know* neither is missing in this case
offset = scan_onsets[0] - physio_onsets[0] # -0.7
new_scan_onsets = scan_onsets + offset
# now we loop through, take each new scan (onset : onset + duration)
# and name the file based on the scan name. There were three things I noticed in my own data that complicated matters:
To deal with these three things, my method is somewhat more complicated than the pseudocode above. Basically, I take the onset delta arrays for the physio and scan data and find the "offset" that minimizes the overall difference between the two arrays. In my data, that overall difference will be > 0. In your data (limited to cases where all scans were retained in the BIDS dataset), that difference should be 0. Of course, the fewer scans there are and the more missing runs there are (both in physio and BIDS), the more likely the method is to be wrong, so that's why it outputs a figure as well. |
Thanks for your detailed reply. |
I extract contiguous blocks of non-zero values in the trigger signal and identify the time of the first point in each block. This step can be divorced from the main workflow code, though, in order to support your type of trigger signal. Per #219 (comment), one good route might be to binarize and "blockify" the trigger signal before the workflow, and retain that simplified trigger in a separate channel. Then the workflow could use the block-form trigger without touching the original trigger. |
From re-reading your last comment, and some of the in-line comments, I think I follow the context and scope of this PR now. Thanks! |
Shall I wait for conflicts to be resolved before properly reviewing? |
@RayStick I've resolved the conflicts so it's good to go. Thanks! |
@tsalo I'm sorry but I really can't see if there was something that I missed and I should have done for this PR. Is it "only" missing new reviews? If so, Maybe someone else could take the lead on this. What is the final solution on the trigger channel? Are you rewriting it or adding another "corrected" or "binarised" trigger channel? |
@smoia See this thread: https://github.com/physiopy/phys2bids/pull/219/files#r455898174. Basically, the method will only work on trigger signals that are already block-ish. I didn't implement a solution for that in this PR, but I did open #278. |
I will take a look on Monday, though bear in mind I have not looked at the phys2bids code for a while now ... so I might be a bit slow to review this! If it needs to be merged sooner, I'd suggest someone else jump in as second reviewer, sorry! |
Docstring rules are way too aggressive. Also, E203 should be ignored. It's wrong.
Codecov Report
@@ Coverage Diff @@
## master #219 +/- ##
===========================================
- Coverage 94.61% 76.26% -18.35%
===========================================
Files 8 10 +2
Lines 836 1083 +247
===========================================
+ Hits 791 826 +35
- Misses 45 257 +212
|
Closes #217. This enhancement adds several functions to do the following:
BlueprintInput
object.BlueprintOutput
s.BlueprintOutput
objects to BIDS files.Current Development/Testing Strategy
BlueprintOutput
objects.To Do
SeriesTime
,InstanceCreationTime
, andContentTime
are very similar, butAcquisitionTime
is generally ~20 seconds earlier than the rest.We need to determine which time is closest to the scanner's trigger pulse.AcquisitionTime is when the trigger pulse is sent, but heudiconv uses ContentTime.AcquisitionTime
instead of waiting foracq_time
in the scans file. Checking this one off.phys2bids
classes and methods for combining and splitting physio data.echo
.Proposed Changes