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

Shifted predictions #113

Open
jcohenadad opened this issue Jul 19, 2024 · 9 comments
Open

Shifted predictions #113

jcohenadad opened this issue Jul 19, 2024 · 9 comments

Comments

@jcohenadad
Copy link
Member

Some predictions are shifted in the axial plane, see notably: valosekj/dcm-brno#19

@valosekj
Copy link
Member

The shift in the top slices is also presented for v2.3.
v2.3 vs v2.4 details: valosekj/dcm-brno#19 (comment)

@naga-karthik
Copy link
Collaborator

Relevant issue ivadomed/canproco#102

@jcohenadad
Copy link
Member Author

jcohenadad commented Jul 22, 2024

One possible cause: the resampling of some highly anisotropic data to 1mm iso. For example, the DWI data from BRNO:
sub-1860B6472B_ses-1860B_acq-ZOOMit_dir-AP_dwi_crop_crop_moco_dwi_mean.nii.gz

We observe a clear shift in the predictions:
image

Interesting observation: when applying the resampling, there is an apparent shift close to the interface between two axial slices:
bad

whereas when the resampling happens in the middle of the slice, there is no apparent shift:
good

Note

The type of interpolation (linear vs. spline) does not make any difference

Next steps:

  • apply the model on the resampled data to see if the shift still occurs.
  • check the inversion (Invertd) to see how the resampled image is brought back to the native space (monai hardcoded this part, so worth checking)

@jcohenadad
Copy link
Member Author

jcohenadad commented Jul 22, 2024

When applying the model on the resampled data, the shift is still visible, at the same location, ie: towards the top slices:

image

Maybe this is related to an 'edge' effect (eg: padding that would somehow shift the prediction of the top slices?)

@naga-karthik
Copy link
Collaborator

naga-karthik commented Jul 24, 2024

I found something really interesting when trying the different padding options available during inference.

Model: contrast-agnostic v2.4 used via SCT's master branch

Padding options tested:

  • constant (pads with a constant value i.e. zero), (this is default applied in SCT)
  • edge (pads with the edge values of array),
  • symmetric (pads with the reflection of the vector mirrored along the edge of the array)

What I changed in the code -- these two lines were replaced with:

        ResizeWithPadOrCropd(keys=["image"], spatial_size=crop_size, mode='edge'), 
        DivisiblePadd(keys=["image"], k=2 ** 5, mode='edge'),
zero-padding -- shift still exists

ezgif com-animated-gif-maker

symmetric (mirror) padding -- shift still exists and also undersegments

ezgif com-animated-gif-maker-2

edge-padding -- shifting resolved! the seg also seems to be centred

ezgif com-animated-gif-maker-3

This is also consistent with my earlier investigations on dcm-zurich and sci-colorado datasets (slides). I think I should set edge padding as default on the SCT's inference script for monai models

EDIT

Also tested on 1mm iso resampled images and it seems that edge padding is able to segment without any shifts

edge-padding on 1mm resampled image

ezgif com-animated-gif-maker-4

@valosekj
Copy link
Member

Nice! 👍🏻

edge-padding -- shifting resolved! the seg also seems to be centred

Do you have a hunch why python monai/run_inference_single_image.py --pad-mode edge did not help in the case of valosekj/dcm-brno#19 (comment)?

@naga-karthik
Copy link
Collaborator

Do you have a hunch why python monai/run_inference_single_image.py --pad-mode edge did not help

I was surprised about this too then I realized that I wasn't applying the padding at the right step. In the run_inference_single_image.py script, the padding is applied only at this line:

DivisiblePadd(keys=["image"], k=2**5, mode=pad_mode),

But turns out that you would have apply the padding in both these lines:

ResizeWithPadOrCropd(keys=["image"], spatial_size=crop_size, mode=pad_mode),
DivisiblePadd(keys=["image"], k=2**5, mode=pad_mode),

in order to have the desired effect! I have fixed that bug in this commit in my new branch.

@naga-karthik
Copy link
Collaborator

The changes required on the SCT side can be accessed if you checkout to my branch https://github.com/spinalcordtoolbox/spinalcordtoolbox/tree/nk/set-edge-pad-default

(will open a PR once it is tested on all dcm-brno subjects and after we see consistent improvement fromedge padding over zero padding

@valosekj
Copy link
Member

Thank you for the fix, Naga! I tested the updated edge padding on dcm-brno DWI images, and the shift is much better!

See comparison on 5 subjects (original padding vs updated edge padding):

Kapture 2024-07-25 at 11 57 15

@naga-karthik, maybe you can open a PR and merge the fixed padding to the SCT master branch?

joshuacwnewton added a commit to spinalcordtoolbox/spinalcordtoolbox that referenced this issue Jul 29, 2024
## Description

Currently, test-time preprocessing transforms for the monai models
(which, at the moment, is only the contrast-agnostic model), used
zero-padding during cropping and padding as in [these
lines](https://github.com/spinalcordtoolbox/spinalcordtoolbox/blob/master/spinalcordtoolbox/deepseg/monai.py#L176-L177).
However, @valosekj observed that zero-padding DWI images from the
`dcm-brno` dataset resulted in [shifted
predictions](valosekj/dcm-brno#19 (comment)).
Interestingly, the shifts were only observed in the initial and final
slices, suggesting that this might be sub-optimal padding issue.

I experimented with different padding options as test-time
[here](sct-pipeline/contrast-agnostic-softseg-spinalcord#113 (comment))
and found that `edge`-padding fixed the issue with shifted predictions.

Hence, this PR updates the default padding (which is zero padding) to
`edge` padding. This change should not break anything as `edge` padding
is at least as good as the zero-padding (so it is not risky to make it
the default). Morever, I noticed that nnUNet has also started to
(subtly) make `edge` padding the default, supporting the changes in this
PR.

---------

Co-authored-by: Joshua Newton <[email protected]>
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

No branches or pull requests

3 participants