Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support liberal mask in IBMA estimators #848

Merged
merged 18 commits into from Dec 14, 2023

Conversation

JulioAPeraza
Copy link
Collaborator

Closes None. Related to neurostuff/PyMARE#44

Changes proposed in this pull request:

  • Add _apply_liberal_mask to separate the data in bags of voxels in the same studies. Some parts of the proposed function require nested for loops, which was inefficient, so I have adapted some of the procedures to use numba with nopython=True.
  • Currently, we loop over the bags and create a Pymare dataset. In the future, we can parallelize that loop to improve performance.

@JulioAPeraza JulioAPeraza added enhancement New feature or request ibma Issues/PRs pertaining to image-based meta-analysis labels Dec 1, 2023
@JulioAPeraza
Copy link
Collaborator Author

JulioAPeraza commented Dec 1, 2023

Some preliminary results using Stouffers:

image

@tsalo
Copy link
Member

tsalo commented Dec 1, 2023

That's so awesome! Do you have a DOF map among the outputs?

@JulioAPeraza
Copy link
Collaborator Author

JulioAPeraza commented Dec 1, 2023

We don't have them now, but that will be a great addition.

Copy link

codecov bot commented Dec 1, 2023

Codecov Report

Attention: 30 lines in your changes are missing coverage. Please review.

Comparison is base (a1c0414) 89.13% compared to head (6dd2e12) 89.10%.

Files Patch % Lines
nimare/meta/utils.py 9.09% 30 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #848      +/-   ##
==========================================
- Coverage   89.13%   89.10%   -0.03%     
==========================================
  Files          49       49              
  Lines        6155     6417     +262     
==========================================
+ Hits         5486     5718     +232     
- Misses        669      699      +30     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JulioAPeraza
Copy link
Collaborator Author

Thanks @tsalo! Would that look like the figure below?

image

@tsalo
Copy link
Member

tsalo commented Dec 2, 2023

I can't access the linked figure, unfortunately.

@JulioAPeraza
Copy link
Collaborator Author

Sorry, I edited the message yesterday, so the figure probably got deleted.
Could you try refreshing the browser (or accessing the PR without the link in the email) and see if the figure shows in the comment above?

@tsalo
Copy link
Member

tsalo commented Dec 2, 2023

I can see it now. It looks awesome!

Copy link
Member

@jdkent jdkent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a couple changes I requested, but otherwise LGTM, will think of possible refactor of some of the class specific bag handling to be placed in the base class.

# This is the same for all voxels in the match
study_mask = study_by_voxels_idxs[voxel_mask[0]]

if len(study_mask) < 2:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set the number 2 to a variable here, like MIN_STUDY_THRESH, defined at the top of the function.

Comment on lines 15 to 157
pytest.param(
ibma.Stouffers,
False,
{"use_sample_size": True},
None,
{},
("z", "p", "dof"),
id="Stouffers_weighted",
),
pytest.param(
ibma.WeightedLeastSquares,
True,
{"tau2": 0},
None,
{},
("z", "p", "est", "se", "dof"),
id="WeightedLeastSquares",
),
pytest.param(
ibma.WeightedLeastSquares,
False,
{"tau2": 0},
None,
{},
("z", "p", "est", "se"),
("z", "p", "est", "se", "dof"),
id="WeightedLeastSquares",
),
pytest.param(
ibma.DerSimonianLaird,
True,
{},
None,
{},
("z", "p", "est", "se", "tau2"),
("z", "p", "est", "se", "tau2", "dof"),
id="DerSimonianLaird",
),
pytest.param(
ibma.DerSimonianLaird,
False,
{},
None,
{},
("z", "p", "est", "se", "tau2", "dof"),
id="DerSimonianLaird",
),
pytest.param(
ibma.Hedges,
True,
{},
None,
{},
("z", "p", "est", "se", "tau2"),
("z", "p", "est", "se", "tau2", "dof"),
id="Hedges",
),
pytest.param(
ibma.Hedges,
False,
{},
None,
{},
("z", "p", "est", "se", "tau2", "dof"),
id="Hedges",
),
pytest.param(
ibma.SampleSizeBasedLikelihood,
True,
{"method": "ml"},
None,
{},
("z", "p", "est", "se", "tau2", "sigma2", "dof"),
id="SampleSizeBasedLikelihood_ml",
),
pytest.param(
ibma.SampleSizeBasedLikelihood,
False,
{"method": "ml"},
None,
{},
("z", "p", "est", "se", "tau2", "sigma2"),
("z", "p", "est", "se", "tau2", "sigma2", "dof"),
id="SampleSizeBasedLikelihood_ml",
),
pytest.param(
ibma.SampleSizeBasedLikelihood,
True,
{"method": "reml"},
None,
{},
("z", "p", "est", "se", "tau2", "sigma2"),
("z", "p", "est", "se", "tau2", "sigma2", "dof"),
id="SampleSizeBasedLikelihood_reml",
),
pytest.param(
ibma.SampleSizeBasedLikelihood,
False,
{"method": "reml"},
None,
Copy link
Member

@jdkent jdkent Dec 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't select all the lines I needed to, but this can be simplified since aggressive_masking is being turned on/off for every IBMA, you can specify another parametrize decorator (see at the bottom, and the ids just name the tests more clearly).

@pytest.mark.parametrize(
    "meta,meta_kwargs,corrector,corrector_kwargs,maps",
    [
        pytest.param(
            ibma.Fishers,
            {},
            FDRCorrector,
            {"method": "indep", "alpha": 0.001},
            ("z", "p", "dof"),
            id="Fishers",
        ),
        pytest.param(
            ibma.Stouffers,
            {"use_sample_size": False},
            None,
            {},
            ("z", "p", "dof"),
            id="Stouffers",
        ),
        pytest.param(
            ibma.Stouffers,
            {"use_sample_size": True},
            None,
            {},
            ("z", "p", "dof"),
            id="Stouffers_weighted",
        ),
        pytest.param(
            ibma.WeightedLeastSquares,
            {"tau2": 0},
            None,
            {},
            ("z", "p", "est", "se", "dof"),
            id="WeightedLeastSquares",
        ),
        pytest.param(
            ibma.DerSimonianLaird,
            {},
            None,
            {},
            ("z", "p", "est", "se", "tau2", "dof"),
            id="DerSimonianLaird",
        ),
        pytest.param(
            ibma.Hedges,
            {},
            None,
            {},
            ("z", "p", "est", "se", "tau2", "dof"),
            id="Hedges",
        ),
        pytest.param(
            ibma.SampleSizeBasedLikelihood,
            {"method": "ml"},
            None,
            {},
            ("z", "p", "est", "se", "tau2", "sigma2", "dof"),
            id="SampleSizeBasedLikelihood_ml",
        ),
        pytest.param(
            ibma.SampleSizeBasedLikelihood,
            {"method": "reml"},
            None,
            {},
            ("z", "p", "est", "se", "tau2", "sigma2", "dof"),
            id="SampleSizeBasedLikelihood_reml",
        ),
        pytest.param(
            ibma.VarianceBasedLikelihood,
            {"method": "ml"},
            None,
            {},
            ("z", "p", "est", "se", "tau2", "dof"),
            id="VarianceBasedLikelihood_ml",
        ),
        pytest.param(
            ibma.VarianceBasedLikelihood,
            {"method": "reml"},
            None,
            {},
            ("z", "p", "est", "se", "tau2", "dof"),
            id="VarianceBasedLikelihood_reml",
        ),
        pytest.param(
            ibma.PermutedOLS,
            {"two_sided": True},
            FWECorrector,
            {"method": "montecarlo", "n_iters": 100, "n_cores": 1},
            ("t", "z", "dof"),
            id="PermutedOLS",
        ),
    ],
)
@pytest.mark.parametrize("aggressive_mask", [True, False], ids=["aggressive", "liberal"])

Copy link
Member

@jdkent jdkent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is some potential repetitive code, but that can be refactored later, LGTM!

@jdkent jdkent merged commit 0e10f03 into neurostuff:main Dec 14, 2023
20 checks passed
@JulioAPeraza JulioAPeraza deleted the ibma-masking branch December 14, 2023 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ibma Issues/PRs pertaining to image-based meta-analysis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants