Skip to content

Conversation

@rijobro
Copy link
Contributor

@rijobro rijobro commented Apr 13, 2022

This shows how I would propose to modify LoadImage to deal with MetaTensor.

This caused a lot of errors throughout our codebase, which I solved with FromMetaTensord wherever necessary.

This is a temporary fix and the usages of FromMetaTensord will be deleted as other parts of the codebase are updated to be compatible with MetaTensor.

This is a breaking change, so the PR is to MONAI/MetaTensor instead of dev.

Status

Ready

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • In-line docstrings updated.

@rijobro rijobro requested review from Nic-Ma and wyli April 13, 2022 16:35
@Nic-Ma Nic-Ma requested a review from ericspod April 14, 2022 04:17
rijobro added 13 commits April 14, 2022 15:36
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro rijobro force-pushed the MetaTensorLoadImage branch 4 times, most recently from 1d57add to 6c4c3d0 Compare April 20, 2022 14:20
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro rijobro force-pushed the MetaTensorLoadImage branch from 6c4c3d0 to f9fd14a Compare April 20, 2022 14:24
rijobro added 2 commits April 20, 2022 15:24
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro rijobro force-pushed the MetaTensorLoadImage branch from 78728d2 to b227fdd Compare April 20, 2022 15:17
rijobro added 5 commits April 20, 2022 16:52
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
rijobro added 6 commits April 21, 2022 15:13
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro
Copy link
Contributor Author

rijobro commented Apr 22, 2022

@Nic-Ma @ericspod @wyli
This branch seems to be working. Also all of the monai tutorials correctly run on this modified branch: https://github.com/Project-MONAI/tutorials/tree/MetaTensor. Could you please review this PR when you have the chance?

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Copy link
Member

@ericspod ericspod left a comment

Choose a reason for hiding this comment

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

Looks good, I have a few optional suggestions.

@wyli
Copy link
Contributor

wyli commented Apr 27, 2022

Hi @rijobro, branches with the name pattern 'feature/*' will trigger more integration tests


after this PR, I'll rename this MetaTensor branch to feature/MetaTensor to benefit from this...

wyli and others added 3 commits April 27, 2022 22:43
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro
Copy link
Contributor Author

rijobro commented Apr 29, 2022

@ericspod thanks for the code review. I've incorporated all suggestions in 9e7b3d6.

@rijobro
Copy link
Contributor Author

rijobro commented Apr 29, 2022

@wyli Any idea what's going wrong with the checks? Some jobs are failing before starting with:

Reading package lists...
W: GPG error: https://developer.download.nvidia.com/compute/cuda/repos/ubuntu1804/x86_64  InRelease: The following signatures couldn't be verified because the public key is not available: NO_PUBKEY A4B469963BF863CC
E: The repository 'https://developer.download.nvidia.com/compute/cuda/repos/ubuntu1804/x86_64  InRelease' is not signed.
Error: Process completed with exit code 100.

@wyli
Copy link
Contributor

wyli commented Apr 29, 2022

@wyli Any idea what's going wrong with the checks? Some jobs are failing before starting with:

Reading package lists...
W: GPG error: https://developer.download.nvidia.com/compute/cuda/repos/ubuntu1804/x86_64  InRelease: The following signatures couldn't be verified because the public key is not available: NO_PUBKEY A4B469963BF863CC
E: The repository 'https://developer.download.nvidia.com/compute/cuda/repos/ubuntu1804/x86_64  InRelease' is not signed.
Error: Process completed with exit code 100.

it's a known NGC issue and the team is working on it, (https://forums.developer.nvidia.com/t/invalid-public-key-for-cuda-apt-repository/212901)

@wyli
Copy link
Contributor

wyli commented May 2, 2022

for the record for more tests, after this PR I'll

(we also need a torch_function benchmark such as: https://github.com/pytorch/pytorch/tree/c371542efc31b1abfe6f388042aa3ab0cef935f2/benchmarks/overrides_benchmark

wyli and others added 3 commits May 2, 2022 13:24
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli
Copy link
Contributor

wyli commented May 3, 2022

/build

@wyli wyli force-pushed the MetaTensorLoadImage branch from be69305 to 56cb925 Compare May 3, 2022 16:52
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli wyli force-pushed the MetaTensorLoadImage branch from 56cb925 to c74f0b3 Compare May 3, 2022 16:59
@wyli wyli merged commit 18fca88 into Project-MONAI:MetaTensor May 3, 2022

def __repr__(self) -> str:
"""String representation of class."""
out: str = super().__repr__()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this line should be reverted, I think the output should be the same as torch.Tensor, but with the extra metadata (now we have metadata instead of voxel data). @wyli what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

sure, please feel free to change, the corresponding test case is this line:

self.assertTrue(str(out).startswith("\nMetaData"))

wyli added a commit that referenced this pull request May 11, 2022
* collate , decollate, dataset, dataloader, out=

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* mypy

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* skip decollation for pytorch 1.7

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* fix

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* fix

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* add batch index testing

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* fixes

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* fix

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* fix

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* fix

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* fix

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* load image meta tensor

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* splitdims fix

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* flake8

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* fix test_nifti_rw

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* test_smartcachedataset

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* test fixes

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* test fixes

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* fix

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* fixes

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* fix

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* fixes

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* fix wsi test

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* changes after code review

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@rijobro rijobro deleted the MetaTensorLoadImage branch May 19, 2022 11:07
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.

3 participants