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

Implement missing restore feature for RandomGhosting #1211

Merged
merged 6 commits into from
Nov 16, 2024
Merged

Conversation

fepegar
Copy link
Owner

@fepegar fepegar commented Sep 23, 2024

Fixes #1196.

Description

As reported by @sravan953, the restore argument in RandomGhosting is unused. This PR addresses this issue.

Checklist

  • I have read the CONTRIBUTING docs and have a developer setup (especially important are pre-commitand pytest)
  • Non-breaking change (would not break existing functionality)
  • Breaking change (would cause existing functionality to change)
  • Tests added or modified to cover the changes
  • Integration tests passed locally by running pytest
  • In-line docstrings updated
  • Documentation updated, tested running make html inside the docs/ folder
  • This pull request is ready to be reviewed

@fepegar fepegar marked this pull request as draft September 23, 2024 21:49
Copy link

codecov bot commented Sep 23, 2024

Codecov Report

Attention: Patch coverage is 80.48780% with 8 lines in your changes missing coverage. Please review.

Project coverage is 84.64%. Comparing base (7b7a732) to head (9b8f119).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ansforms/augmentation/intensity/random_ghosting.py 80.48% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1211      +/-   ##
==========================================
- Coverage   84.77%   84.64%   -0.13%     
==========================================
  Files          92       92              
  Lines        6023     6026       +3     
==========================================
- Hits         5106     5101       -5     
- Misses        917      925       +8     

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

@fepegar
Copy link
Owner Author

fepegar commented Sep 23, 2024

@romainVala @sravan953 I've implemented the feature as expected, but I'm not sure it's very useful. What do you think? Here's an example:

from itertools import product

import numpy as np
import torchvision.transforms as T
from PIL import Image
from torchvision.utils import make_grid

import torchio as tio

t1 = tio.datasets.FPG().t1
t1 = tio.ToCanonical()(t1)

restores = None, 0.1, 0.25, 0.5, 0.75
intensities = 0.25, 0.5, 0.75, 1

images = {}
t1_slice = t1.data[0, :, :, t1.shape[2] // 2].float()
min_data, max_data = t1_slice.data.min(), t1_slice.data.max()
for intensity, restore in product(intensities, restores):
    print(f'intensity={intensity}, restore={restore}')
    if restore is None:
        restore_ = None
    else:
        restore_ = restore, restore
    transform = tio.Ghosting(
        num_ghosts=2,
        intensity=intensity,
        restore=restore,
        axis=1,
    )
    transformed = transform(t1)
    t1_slice = transformed.data[0, :, :, transformed.shape[2] // 2].float()
    t1_slice = (t1_slice - min_data) / (max_data - min_data) * 255
    # t1_slice = (t1_slice - t1_slice.min()) / (t1_slice.max() - t1_slice.min()) * 255
    array = t1_slice.clamp(0, 255).numpy().astype('uint8')
    array = np.rot90(array)
    image = Image.fromarray(array)
    images[intensity, restore] = image

images_list = list(images.values())
images_list = [T.ToTensor()(image) for image in images_list]
grid = make_grid(images_list, nrow=len(restores))
T.ToPILImage()(grid)

image

@romainVala
Copy link
Contributor

romainVala commented Sep 24, 2024

Hi thanks Fernando for a quick fix

(and nice plot !)

It is not clear to me how "physical" in this implementation of MRI ghosting effect
(In EPI this is the phase difference from one line to an other that cause the ghosting)

So difficult to say if (from a physical point of view) one should keep this parameter

But from a visual point of view, this restore parameter make sense (it remove the low frequency part of the ghost, ) and so it produce more variability of the artefact

So I think it is good to keep !

many thanks

@fepegar fepegar marked this pull request as ready for review November 16, 2024 22:35
@fepegar fepegar merged commit 524ad75 into main Nov 16, 2024
24 of 25 checks passed
@fepegar fepegar deleted the fix-random-ghosting branch November 16, 2024 22:36
@fepegar
Copy link
Owner Author

fepegar commented Nov 16, 2024

@allcontributors please add @sravan953 for bug

Copy link
Contributor

@fepegar

I've put up a pull request to add @sravan953! 🎉

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

Successfully merging this pull request may close these issues.

RandomGhosting: restore parameter has no effect
2 participants