Implement RABIES

[david-roper]

The following pull request is a draft to provide a centralized point for discussion and tracking of the implementation of the RABIES pipeline into nipoppy.

More information about the RABIES fmri pipeline is provided at the following sites:

Github (https://github.com/CoBrALab/RABIES)
RABIES documentation (https://rabies.readthedocs.io/en/stable/)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Gab-D-G]

was this ever tested on a sample dataset?

[david-roper]

@Gab-D-G This implementation was ran with the sample dataset provided in the rabies tutorial , specifically the one available on zenodo

[Gab-D-G]

@Gab-D-G This implementation was ran with the sample dataset provided in the rabies tutorial , specifically the one available on zenodo

Great! If all ran without errors, then it should be all good.

[Gab-D-G]

Just make sure that the RABIES default are not overridden. If its not the case, then all is good on my end.

[david-roper]

@michellewang @Gab-D-G I was wondering how I should go about creating the pipeline tracker configuration file for RABIES and if it is necessary to implement? The final output of the pipeline can have 60+ files, which could lead to quite a large tracker config file if they were all to be tracked. In addition, some files will only exist if a certain argument is passed (ex: the --data_diagnosis tag). Any advice how to tackle this would be greatly appreciated, thanks!

[Gab-D-G]

you are right @david-roper that RABIES outputs lots of files, and the set of file varies according to the input parameters. It would be preferable that it is not necessary to track every file manually, since nipype is doing so internal when running RABIES... Not sure I can advise further on this end.

[michellewang]

Hi @david-roper, the tracker configuration file is not required, so it's fine if there isn't one for RABIES. Though just to clarify, the tracker doesn't need to include an exhaustive list of all expected output files. If there is a set of files (or a single file) that is generally important for downstream analysis (and are produced no matter the input arguments), then you could include it in the tracker config file, though again that is not required.

Another thing: trackers currently are specified by pipeline, not pipeline step (we might change that, see #258). So if the steps in RABIES are sequential then it might make sense to track only the expected outputs of the last step.

FYI we're currently thinking about how to manage the growing library of supported pipelines (see #316), as it doesn't necessarily make sense to have the JSON descriptors/invocations all in the main code repo. I think we might want to wait until we settle on long-term solution for this before merging this PR.

In the meantime if you have any feedback about the docs or anything else, please let us know!

[david-roper]

Ok great thank you both for the advice. I will hold merging the descriptor and invocation files for now until a long term solution (Zenodo upload, etc.) is set (#316). I will also keep the tracker config empty for now but will do a deeper dive into the important files from the final output of RABIES (i.e. the analysis step) that would be beneficial to track. I found the current docs quite helpful and if I have any extra suggestions for them I will let you know!

[michellewang]

Thanks @david-roper! Two comments:

1. We don't need invocation/descriptor files for the *-tracking steps. It is okay (and preferred in my opinion) to have only the tracker config files for them
2. It might be better to allow for globbing in the tracker paths (i.e. use * instead of hardcoded BIDS entities like run)

Otherwise looks good! Let me know if you have any questions!

[david-roper]

@michellewang Thank you for the suggestions! I have implemented them into code. Let me know if there's anything else needed. Thanks again for all the help on this pr!

[michellewang]

I just have some remaining questions regarding the tracker config for the analysis step. Once these are addressed we can finally merge this :)

[david-roper]

@michellewang I have added the last changes to the pr. However it seems that the docs build test is failing within the CI now. Would pulling the most recent changes from main resolve this? Thanks!

[michellewang]

@david-roper the docs failed because of a timeout in the linkcheck step, I restarted the build and it worked just now. Merging from main might fix it because we increased the timeout limit

[david-roper]

@michellewang Everything seems to be in working order now. The tracker configs all have successful readings in the imaging_bagel.tsv when run. As the all the regular steps of the rabies pipeline run successfully. If there is anything else needed to be added let me know. Thank you yet again for all the help on this PR!

[michellewang]

Looks good @david-roper! I will go ahead and merge this, though I also wanted to let you know that we are working making a big change to the way pipeline configs work with Nipoppy. Briefly:

• Currently, pipeline updates or contributions require a PR to the nipoppy/nipoppy repo, and new changes will only be obtainable by regular users when Nipoppy makes a new release, which is not ideal
• So we want to decouple the sample Nipoppy pipelines from the core Nipoppy CLI code
• Instead of being in the sample_pipelines directory, the currently supported pipelines will be uploaded to Zenodo and look something like this. The Nipoppy CLI will have new commands for uploading/downloading pipelines from Zenodo
• This means that for example if you ever need to change the RABIES config, you can update the Zenodo record without going through this PR process (though we will still be happy to give feedback on pipeline configs if needed), and the new record will be usable immediately by other Nipoppy users

This is actively being worked on in the dev-catalog branch and will be a breaking change once merged/released. For RABIES specifically you will need to change 3 things:

• Move the RABIES part of the global config into a config.json file inside the rabies-0.5.1 directory
• Update the paths to the descriptor/invocation/tracker config files in config.json so that they are relative to rabies-0.5.1/ (i.e. remove the [[NIPOPPY_DPATH_PIPELINES]]/[[PIPELINE_NAME]]-[[PIPELINE_VERSION]]/ part).
  Then run something like nipoppy pipeline upload /path/to/rabies-0.5.1 with a Zenodo API token
• Add a field "PIPELINE_TYPE": "processing" to config.json

Let me know if this makes sense and/or if you have any questions. I will let you know when the Zenodo things are ready (hopefully by the end of the month)