bbfrederick / rapidtide

rapidtide - a suite of programs for doing time lag correlation analysis on fMRI data
Apache License 2.0
75 stars 14 forks source link

Continue to perform regressor refinement until regressor convergence #50

Closed robbwh closed 3 years ago

robbwh commented 3 years ago

Is your feature request related to a problem? Please describe. As far as I can tell, there is currently no way via command line arguments to dynamically set the number regressor refinement passes. It might be good to continue refinement until regressor convergence.

Describe the solution you'd like This paper uses the RIPTiDe method and describes performing regressor refinement until the mean-squared error between the regressor probes is below a threshold of 0.0005

Ideally there would be an argument such as --convergethresh that would dynamically set the number of regressor refinement passes.

bbfrederick commented 3 years ago

That's a good idea. I'll put that in.

tsalo commented 3 years ago

One minor request that fits with this. If you could add a --maxpasses argument as well, that would be great. Then, if the maximum number of passes are reached without convergence, rapidtide could log a warning, but stop the refinement procedure. A special value of -1 or 0 could mean to keep going until the threshold is reached, I guess, but that seems like a recipe for problems.

bbfrederick commented 3 years ago

Yeah, that would be necessary, since it's possible it could oscillate around a solution forever.

bbfrederick commented 3 years ago

Ok, the feature has been added to 2.0alpha7. By default, rapidtide will run for the number of passes that you specify. However, if you use the command line option "--convergencethresh THRESH", it will continue to run until either 1) the change in mean square difference between one regressor and the next falls below THRESH or 2) you reach maxpasses (15 by default, but you can specify what you want with "--maxpasses PASSES"). If you want to exactly replicate the Champagne, et al procedure, you should also use "--globalmeaninclude GRAYMATTERMASK.nii.gz" and "--refinemask GRAYMATTERMASK.nii.gz". Give it a try and let me know what you think.

Also, the default refinement procedure has been changed from "unweighted_average" to "pca", and the PCA procedure has been modified to replicate Champagne's method (retain enough components to model 80% of the variance). You can change the variance target using the "--PCAtarget FRAC" argument, where FRAC is the fraction of variance explained - 0.8 by default.

I'm planning to modify tidepool so that instead of showing the first 4 passes of regressors, it will show all regressors if there are 4 or fewer passes, and the first two and last two if the number of passes is greater than 4.

tsalo commented 3 years ago

I know that this probably deserves its own issue, but will you deprecate/remove the --passes argument now that --convergencethresh and --maxpasses are implemented?

bbfrederick commented 3 years ago

My thought was no - I think there's some advantage to being able to run for a fixed number of passes. I know you could do it by adjusting maxpasses, but I'd like to keep those options separate. I could be convinced otherwise, however.

tsalo commented 3 years ago

My concern is that rapidtide currently has dozens of arguments. The biggest problem with having so many arguments is that it tends to overwhelm users (as I can attest to myself), but it also opens up opportunities for the arguments to interact in undocumented or unpredictable ways. For example, the fact that passes supersedes maxpasses if convergencethresh is not set is only documented in the convergencethresh description.

It also seems like using maxpasses in lieu of passes when convergencethresh is not set, as you mentioned, would be fairly straightforward for users.

robbwh commented 3 years ago

The new feature seems to be working! Thanks for the prompt addition of it!