Agenda for HRF analysis

[rob-luke]

Here is a rough outline of plans and issues with this HRF style analysis as currently implemented.

Overview

This tutorial shows the current status. https://mne.tools/mne-nirs/auto_examples/plot_10_hrf.html

Things work, but the code API needs improvement and a better topomap code is required.

Code base

I am basing the analysis on Nistats which includes very similar analysis to what I had planned. Nistats is currently being ported to nilearn.

• How can I make PRs in the meantime as nistats repo has been archived and porting won't be completed till late 2020.
• What is the plan for porting to nilearn? Will everything be included? Is there a way I can ensure the features used here are kept?

There are some things not included in Nistats that I would like to eventually contribute including:

• More granular specification of HRFs such as overshoot, offsets, etc Already implemented in Nilearn
• An iteratively reweighed approach to the AR process

Current status of code

The code is quite rough now, but demonstrates the analysis approach I wish to take.

Outstanding Issues

• How do I make a nicer wrapping function for the topoplots to be more like other MNE types? Currently I have a complete hack of a job. I don't even have a colorbar yet! Moved to own issue at Add colorbar to plot_glm_topo #72

• Testing just checks this runs. Need to add proper tests. Maybe some simulation then I can check the correct values are estimated. Moved to Add testing with Azure pipelines #56
• The code is taking up too much RAM to run a full analysis in the GitHub runner. It seems to be due to the AR code in Nistats. So currently analysing just the first third of the measurement in the example. Either I move to a different CI or make the code more memory efficient. Discussion moved to PR WIP: Remove crop from HRF example #64

At what point do I move this in to MNE?

MNE has strict depreciation requirements, so I feel I should get the API working smoothly before opening a PR. On the other hand, opening a PR early will get helpful feedback from the experts.

Plans

I will keep chipping away at this over time. Any input is appreciated.

---

mne-tools/mne-python#7057

FYI: @larsoner @agramfort

[larsoner]

MNE has strict depreciation requirements, so I feel I should get the API working smoothly before opening a PR

Indeed. And apparently nistats is moving into nilearn. I was going to say that we might want to wait for that to complete and they release, but:

It will be available in Nilearn 0.7.0 onwards, set to release in late 2020. Please file issues and pull requests in Nilearn, from now on.

Our next MNE release will be in ~Sept, so I'm not sure we can wait for nilearn 0.7.0. I suggest we wait until June or July and assess where the workflow is then. @rob-luke if you (and hopefully others around you?) are going to be using this daily in your work, the API will probably mature by your experiences in the meantime.

If the API here is ready to go (and be reworked if necessary) to go in MNE-Python, we should just do it at that point. We can suffer a little bit of pain internally maintaining code for both nistats and nilearn for one release, backporting the necessary try/except clauses, which hopefully will not be so common or complex anyway.

@rob-luke WDYT? @agramfort sound okay to you?

[larsoner]

What is the plan for porting to nilearn? Will everything be included? Is there a way I can ensure the features used here are kept?

I would expect that HRF, etc. are core concepts that must be ported. I would start by looking at their dev docs to see what has already been ported. Also maybe subscribe to their repo. A quick search of "nistats" in the nilearn repo shows this PR as maybe being relevant:

nilearn/nilearn#2359

There are some things not included in Nistats that I would like to eventually contribute including:

That's a great idea, I would open an issue on Nistats (after searching to see if there is anything similar). It will also implicitly answer your question about how much HRF code will be ported...

Either I move to a different CI or make the code more memory efficient.

You can change which image is used in CircleCI. I would do that for now to get things working. As @agramfort says, make it work, make it nice, make it fast (in this case, memory efficient). Once you have something working with tests and everything you can more easily recruit people who enjoy digging into efficiency issues (myself included) without risking them blowing up the code.

How do I make a nicer wrapping function for the topoplots to be more like other MNE types?

I would live with the hack for now. Stuff like this we can deal with in the summer when we go to integrate in MNE-Python. In the meantime I would say make whatever grotesque shims you need to get the job done :)

[agramfort]

+100 make it work/nice/fast in mne-nirs and we'll see what should be done in mne

…

[rob-luke]

Thank you both. I will continue development of this analysis in MNE-NIRS and ping you both once it has matured.

[rob-luke]

And scratch my first suggestion for what I would add to Nistats. Nilearn already has more advanced HRF models https://github.com/nilearn/nilearn/blob/master/nilearn/stats/first_level_model/hemodynamic_models.py

[rob-luke]

And NIlearn already has the design matrix tools I need from NIstats. So I think I will switch to Nilearn.

https://github.com/nilearn/nilearn/blob/223c4e135568e9bfd766b8fd5d58e195d2e7781e/nilearn/stats/first_level_model/design_matrix.py#L243

[larsoner]

Awesome! That should make transitions much easier

[kchawla-pi]

Hi @rob-luke . I incorporated Nistats into Nilearn. I think you have already discovered this; none of the Nistats features were abandoned during the transition. Some were reorganized in terms of the imports. The PR referenced earlier (nilearn/nilearn#2359) is preparing a documentation to guide this.

In fact, I would appreciate your feedback on it, so it is good quality for other historic Nistats users.

Additionally, most Nistats imports (except reporting and datasets ones) should work seamlessly by replacing nistats with nilearn.stats. Many former nistats imports are now done directly from the nilearn.stats sub-package instead of nilearn.stats.<submodules> but the latter should still work, even if they won't be the officially documented import paths.

So most of your compatibility shims should be fairly simple.

I would suggest, for now, cloning & installing Nilearn's master branch in one of the test configs and Nilearn 0.6.x + Nistats for another configuration to test your shims easily.

[bthirion]

Hi,
I'm happy to hear more about the memory issues you had with Nistats (now nilearn.stats). We have tried to keep memory consumption under control (see minimize_memory argument of FirstlevelModel).
Also, don't hesitate to raise issues or contribute upstream to nilearn if you feel things can be improved. Best,

[rob-luke]

Dear @kchawla-pi and @bthirion,
Thank you for taking the time to comment above. Over the coming days I will port the MNE-NIRS code over to Nilearn and pay special attention to the docs and memory consumption. I will report back my observations, and contribute upstream where I can.
Cheers,

[rob-luke]

The general approach is complete. Have moved outstanding issues to more bite sized pieces.