search

HBClab / NiBetaSeries

Nipype implementation of BetaSeries Correlations (Beta)
https://nibetaseries.readthedocs.io
MIT License
32 stars 28 forks source link

Additional methods to potentially support #197

Closed tsalo closed 5 years ago

tsalo commented 5 years ago

Is your feature request related to a problem? Please describe. LSS is the most popular choice for beta series modeling, but there are a couple of other models that are perhaps more appropriate for different kinds of analysis or with different kinds of data.

Describe the solution you'd like I would like to see the addition of a couple of other model types, possibly including least squares- all (LSA), finite BOLD response- separate (FS), and shifted least squares (SLS). I believe that there are plans to implement LSA, per #156, but FS may be useful for classification analyses when the specified HRF is not very accurate and the results from the SLS poster looked promising (not sure if there is an associated paper yet).

Describe alternatives you've considered These methods could be implemented elsewhere, since NiBetaSeries is primarily geared toward connectivity analyses over classification analyses. However, I think that the workflow is well poised for classification analysis given that LSS was originally developed for classification and that the beta maps will be retained, per #195.

Additional context N/A

tsalo commented 5 years ago

I believe that FS should be pretty easy to implement, since it's essentially LSS with FIR. FIR is an hrf-model option. Only a couple of things will need to be changed/updated for it to work, I think:

  1. Timing and number of the FIR delays need to be set (either hardcoded or allowed as an argument to the workflow).
  2. Contrast computation (to derive the beta maps) needs to support the new regressor names (e.g., congruent_0, congruent_1, etc. instead of congruent).
  3. The organization and shape of the beta series needs to be decided upon. Instead of XxYxZxN (N = number of trials), there are going to be XxYxZxNxF (F = number of FIR delays). 5D images are probably not a good idea, so then the beta series need to be split into sets of 4D images. I'm thinking they should be split by delay.
tsalo commented 5 years ago

I have a working implementation of FS on this branch. It is triggered by using the fir HRF model and using a new argument for the FIR delays (fir_delays). The resulting beta series maps are split by delay, so the files end up being like:

sub-001_task-stroop_space-MNI152NLin2009cAsym_desc-congruentDelay0_betaseries.nii.gz sub-001_task-stroop_space-MNI152NLin2009cAsym_desc-congruentDelay0_correlation.svg sub-001_task-stroop_space-MNI152NLin2009cAsym_desc-congruentDelay0_correlation.tsv sub-001_task-stroop_space-MNI152NLin2009cAsym_desc-congruentDelay1_betaseries.nii.gz sub-001_task-stroop_space-MNI152NLin2009cAsym_desc-congruentDelay1_correlation.svg sub-001_task-stroop_space-MNI152NLin2009cAsym_desc-congruentDelay1_correlation.tsv sub-001_task-stroop_space-MNI152NLin2009cAsym_desc-congruentDelay2_betaseries.nii.gz sub-001_task-stroop_space-MNI152NLin2009cAsym_desc-congruentDelay2_correlation.svg sub-001_task-stroop_space-MNI152NLin2009cAsym_desc-congruentDelay2_correlation.tsv

@jdkent WDYT?

jdkent commented 5 years ago

I think that naming scheme looks reasonable given our constraints. The only other piece of information I'm on the fence about adding is a unit for the number. One could get confused and think, 0, 1, 2 mean seconds and not volumes.

so desc-congruentDelay0 could be desc-congruentDelay0Vol. But that is even uglier in my opinion.

Unless some inspiration strikes me, I believe your naming scheme is the best option currently.

PeerHerholz commented 5 years ago

Thanks for implementing this @tsalo. Coming from auditory neuroscience were FIR models are frequently used, this is a very valuable extension.

Re file naming: I agree with @jdkent. Is it necessary to write out the entire FIR information? Maybe it could be shortened to sub-001_task-stroop_space-MNI152NLin2009cAsym_desc-ConDelayVol0_betaseries.nii.gz or something like that? Not that it is so much different from what has been proposed by you folks... I guess if it's documented accordingly, it should be okay, no?

tsalo commented 5 years ago

@PeerHerholz I'm glad FS will be useful! I haven't seen it anywhere outside of the original Turner paper, but the results were definitely promising.

I can add Vol, but I don't think it's feasible to shorten the condition names automatically. Condition names may overlap, and changing the names may make it harder for users to index the files.

PeerHerholz commented 5 years ago

@tsalo, I'm very sorry, I didn't pay enough attention and forgot that the Congruent part refers to the task. You're completely right in that task/condition ids should not be altered. Sorry for the confusion.

tsalo commented 5 years ago

I don't think that shifted least squares is viable to implement until there's at least a paper, so is everyone comfortable with me closing this? Unless there are other methods out there that might be good to implement.

jdkent commented 5 years ago

I'm good with closing this. from now on, we can open issues that are tied specifically to the method that we wish to implement (easier to maintain the 1:1 ratio between issues and pull requests).

tsalo commented 5 years ago

That's fair. In the future I'll try to refrain from opening sprawling issues like this one.