Summary

This stems from #46 (comment). Basically, the path we use in the CircleCI config for store_artifacts isn't where the integration tests are writing their outputs to, which means we can't access those outputs.

Next Steps

1. Figure out the appropriate output directory to use in the integration test.
2. Use that path.
3. Ensure that store_artifacts is also pointing to that directory.
4. Check that artifacts are accessible.

Printing is fragile- we should log our messages with the logging module.

Summary

I was trying the plotting part and seaborn was not installed in my computer i checked and is not in the setup.cfg

Additional Detail

Next Steps

• add seaborn to the installation dependecies

I am not all that familiar with the requirements of the Apache 2.0 license (the one used by the main ICA-AROMA repo), but we can modify and redistribute the code, with some caveats. We need to incorporate information about the changes we have made into the source code and retain the original license. I'm a little fuzzy on the specific details, but we should review the license to figure out what exactly we need to do.

We should restructure the ICA outputs that ICA-AROMA ingests to be BIDS Derivatives-compatible. In order to maintain support for MELODIC, I think we need a new function that converts MELODIC outputs to a BIDS Derivatives outputs. The main ICA-AROMA functions would then just use BIDS Derivatives outputs.

Summary

Badges currently point to old repo. Need to update path to new repo.

Additional Detail

Next Steps

Currently, ICA-AROMA only supports standard-space inputs (MNI152 with 2mm3 voxels, I believe). In order to maximize compatibility with other ICA classification methods (e.g., tedana), we need to support native-space inputs as well.

Here are some blockers on this:

1. The thresholds in the classification procedure are hardcoded and linked to a specific standard space template.
2. We need methods to derive our masks of interest (brain, edge, csf, and out-of-brain) in native space. I think we need to assume there are tissue probability maps available.

Summary

This issue aims to host a list of tasks/goals for Brainhack Donostia 2021.

As of the start of Brainhack Donostia 2021, we have 11 issues and 5 PRs open. I have added labels to the open issues to indicate the necessary effort, the impact, and the priority of closing them. Feel free to adjust them if they do not seem realistic.

There are also a few issues labeled Good First Issue that should be quite straightforward to get done:

#3 , #42 and #44 (this one actually has PR #45 open).

At the same time, we should try to close the PRs we have open, especially #22 , #28 and #46 . I believe it would be great if we could merge these 3 PRs.

What do you think @tsalo @handwerkerd ?

I'd suggest pulling out the MELODIC node of fMRIPrep as run by CircleCI on ds005 (because this dataset is heavily downsampled, so the computational burden is not terrible).

fMRIPrep should also generate the necessary TPMs in native space.

Steps:

1. Modify the ICA-AROMA code, making sure all intermediate results are not wiped out.
2. Run that derived code from ICA-AROMA locally with FSL installed
3. Create a zipfile with intermediate results, including all the masks generated along the way as well as the conversion (resampling) of the initial BOLD in standard space. Make sure all the masks that the feature extraction requires are all kept in the standard space. Include in the zip the dataframe of features you obtain (could be done together with #31)
4. Write unit tests of the features module using these masks.

We have a set of outputs of interest from AROMA, including:

• Denoised 4D image, in native and standard space (possibly)
• Metrics (feature scores)
• Component time series
• Component maps, in native and standard space (possibly)
• Classifications (included with feature scores in "classification_overview.txt")
• Figure

We should figure out how best to make those outputs BIDS Derivatives-compatible.

At minimum, I think we can use filenames that are basically BIDS-ish, like what I propose in ME-ICA/tedana#574. This means using entities and suffixes that match BIDS convention, minus the "source entities" from the original files (e.g., sub, ses, run).

I don't know if we want to output the classifications/metrics as a json (as in tedana) or a tsv. A tsv would be easier to read...

Summary

We should recognize the contributions made to aroma.

Next Steps

• Add the all contributors bot to update the README.

Summary

To reduce the number of temporary files we may need to write, we should allow the feature functions to accept arrays in addition to files.

Additional Detail

Next Steps

1. Check datatypes of inputs to feature functions. If str, load the file. If array, use that directly.
2. Update the docstrings.
3. Add tests for both types of inputs.

Eliminate these lines:

https://github.com/Brainhack-Donostia/aroma/blob/3c077ecef03cd44fb2d50d7dbabb37d295afb459/aroma/aroma.py#L219-L220

replacing this with a function that reads in the BOLD data and components and returns the representations necessary in the following steps.

The current version uses fslmaths for a number of steps that could very easily be done with nilearn or nibabel + numpy.

The only difficult part that I can think of is handling MELODIC's mixture modeling. We can either (1) assume the MELODIC data are in BIDS format already and work from there or (2) try to "implement" the mixture modeling method with a Python-based ICA algorithm.

Summary

Additional Detail

Next Steps

• Bring automatic tests from the original repo.

Steps:

1. Run the original ICA-AROMA locally with FSL installed
2. Patch in a new classifier function where the input dataframe is written out to hard-disk, as well as the list of motion/no-motion labels.
3. Add the dataframe to this repo
4. Write a unit test of the predict function using this dataframe + labels

Summary

We should add contributing information and a code of conduct to make things easier for new contributors.

Additional Detail

We can base our information on tedana, which is very welcoming.

Next Steps

1. Add contributing guidelines and code of conduct, adapted from tedana.

Summary

We could make the package much lighter by moving the data we use for testing into an OSF repository.

Next Steps

• Create an OSF repo for aroma under ME-ICA.
• Upload files under tests/data to the OSF repo.
• Write a function to download files for testing in conftest.py.
• Remove the tests/data directory.

Summary

In a couple of locations, tabular data (e.g., feature scores and classifications) are operated on as numpy arrays and are written to files using basic _io.TextIOWrapper.write(). We should use pandas, which will clean the code up considerably and will also increase confidence in comparing separate arrays.

Summary

I understand our objective is not u use FSL, however I think that at somepoint we need to setup a test that certifies our AROMA and FSL AROMA to be equivalent. Therefore I propose to find a docker image that can be able to run the integration test that we have now so later is easier to create this compatibility test.

-->

Next Steps

• Look for an fsl docker image for example: https://hub.docker.com/r/bids/base_fsl/
• Try to run the integration tests on it

Summary

This stems from Brainhack-Donostia/ica-aroma-org#41 (comment). We will drop support for native-to-standard space transforms for now, and all native-space data must be accompanied with tissue probability maps for now.

Next Steps

1. Remove all transforms.
2. Drop FEAT directory support too, maybe?

Summary

We should add unit test jobs for Python 3.8 and 3.9. I'm not sure if integration tests are necessary.

Next Steps

1. Add the new jobs to the CircleCI config file.
2. Run the tests to ensure that they pass.

Summary

Running aroma from the command line raises a The folder does not exist error.

Additional Detail

Command I ran:

aroma -i sub-pixar123_task-pixar_space-MNI152NLin2009cAsym_desc-preproc_bold.nii.gz -tr 2 -out out -mc mc.tsv

Error I got:

usage: aroma [-h] -o OUT_DIR (-i IN_FILE | -f IN_FEAT) [-mc MC] [-a AFFMAT] [-w WARP] [-m MASK]
             [-tr TR] [-den {nonaggr,aggr,both,no}] [-md MEL_DIR] [-dim DIM] [-ow] [-np]
aroma: error: The folder  does not exist!

Next Steps

The error is raised by the following line in àroma/cli/aroma.py`:

options = _get_parser().parse_args(argv)

Summary

Do we want to develop Sphinx-based documentation?

Additional Detail

We can build our documentation on a ReadTheDocs site, but I'm not sure if we want to do that in this repository or in a new repo that wasn't created for a demonstration.

Next Steps

1. Copy documentation structure from an existing library (e.g., tedana, NiMARE, or phys2bids).
2. Adapt documentation to fit AROMA.
3. Set up RTD site.

AFAICT, we have a lot of hands, but scattered on many objectives:

• @tsalo is working on several fronts: CLI, mixing model, docstrings/refactors, etc.
• @vinferrer is working on the plotting
• @eurunuela and @CesarCaballeroGaudes are more involved in the coordination and bigger picture.

Should we define some list of priorities? Maybe using the project's milestones or a "Project" with the kanban columns?

It feels like we are trying to pin down too many targets. WDYT?

Summary

The classification plot code uses seaborn, but doesn't take advantage of the available features. We should review the plotting code and identify places where it can be improved.

Additional Detail

An example- instead of using seaborn's hue argument, the classification plot code duplicates the same lines for the accepted and rejected components. This also requires the code to add fake rows in cases where all components are either rejected or accepted.

Next Steps

1. Review the classification plot code for unnecessary duplication.
2. Fix up seaborn usage.

Summary

This came up in today's tedana devs call. We think that it might make it easier to continue development of the AROMA refactor if it's under the ME-ICA organization.

Summary

We should change the docker image for the testing since we are not using fsl anymore.

Additional Detail

Next Steps

1. Chose new image (miniconda?)
2. Implement necessary changes in config.ylm
3. Check tests work in the new image

We can include motion parameter converters for typical outputs, like AfNI, fMRIPrep, and SPM, in addition to the default FSL outputs.