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

User warning when data is being upsampled when masked #3397

Closed
FrancoisPgm opened this issue Oct 25, 2022 · 6 comments · Fixed by #3631
Closed

User warning when data is being upsampled when masked #3397

FrancoisPgm opened this issue Oct 25, 2022 · 6 comments · Fixed by #3631
Labels
Enhancement for feature requests

Comments

@FrancoisPgm
Copy link
Contributor

The default beahaviour of maskers is to resample the data to the mask's affine, but when the data has a lower resolution than the mask, it upsamples the data, requiring more computation and memory than necessary. Throwing a warning when it happens could help users to specify a target affine when it is relevant.

Benefits to the change

Users can realise when they upsample their data, and that they can save computation time and memory by specifying a target affine (or a resampling strategy).

Pseudocode for the new behavior, if applicable

## in transform_single_imgs
if imgs.resolution < self.mask_img_.resolution:
    raise UserWarning("Images are being upsampled to the mask's resolution, "
        "you might want to provide a target_affine to save memory and computation time.")
@FrancoisPgm FrancoisPgm added the Enhancement for feature requests label Oct 25, 2022
@jeromedockes
Copy link
Member

we can improve the warning message but I like the idea!

@FrancoisPgm
Copy link
Contributor Author

Actually now that I'm trying to write a check to catch the upsampling, I'm not so sure anymore that it's always the default behaviour. It for sure has happened to me but I might need to investigate deeper into how I got it to happen.

@alexisthual
Copy link
Contributor

I'm going to see this warning so often haha 😆

@ymzayek
Copy link
Member

ymzayek commented Nov 2, 2022

Hi @FrancoisPgm sorry it took me a while to get back to you. So the resampling to the mask is indeed the default behavior of the NiftiMasker class (and MultiNiftiMasker by inheritance). These maskers use a function called _filter_and_mask in nilearn.maskers.nifti_masker.py to call another function to perform resampling and timeseries extraction. An appropriate place to add your check and warning would therefore be under this _filter_and_mask function. More precisely, in line 103 of nifti_masker.py there is a check if resampling is necessary, you can put your code under this conditional.

It is the case that the default behavior for the other maskers is different and they do not need this warning.

@ymzayek
Copy link
Member

ymzayek commented Mar 24, 2023

@mtorabi59 can you also have a look at this? I think implementing this warning can only be helpful in the meantime while we haven't made a decision about making this behavior consistent across all maskers.

@mtorabi59
Copy link
Contributor

@ymzayek sure, I will put it at the top of my list!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement for feature requests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants