Skip to content

Implement TiffFile backend for WSIReader#4972

Merged
wyli merged 15 commits intoProject-MONAI:devfrom
bhashemian:wsireader-tifffile
Aug 31, 2022
Merged

Implement TiffFile backend for WSIReader#4972
wyli merged 15 commits intoProject-MONAI:devfrom
bhashemian:wsireader-tifffile

Conversation

@bhashemian
Copy link
Copy Markdown
Member

@bhashemian bhashemian commented Aug 23, 2022

Fixes #4900

Description

This PR implements TiffFileWSIReader as the backend of monai.dadta.wsi_reader.WSIReader.

Status

Ready

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

@bhashemian bhashemian requested a review from Nic-Ma August 23, 2022 12:40
@Nic-Ma Nic-Ma requested a review from yiheng-wang-nv August 23, 2022 13:04
@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Aug 23, 2022

Hi @yiheng-wang-nv ,

I think this PR should be useful for your pathology challenge pipeline.
Could you please help review it?

Thanks in advance.

@bhashemian bhashemian enabled auto-merge (squash) August 23, 2022 14:27
@yiheng-wang-nv
Copy link
Copy Markdown
Contributor

Hi @yiheng-wang-nv ,

I think this PR should be useful for your pathology challenge pipeline. Could you please help review it?

Thanks in advance.

Hi @Nic-Ma , I'm not an expert of pathology, and may not be able to review this PR. BTW, I think it may be transplanted from the deprecated class, thus should not have other issues. Could you please invite someone else to review it? Thanks!

Copy link
Copy Markdown
Contributor

@Nic-Ma Nic-Ma 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 to me.
@KumoLiu Could you please help also review it?

Thanks in advance.

@Nic-Ma Nic-Ma requested a review from KumoLiu August 24, 2022 14:11
@wyli
Copy link
Copy Markdown
Contributor

wyli commented Aug 24, 2022

/build

@wyli wyli disabled auto-merge August 24, 2022 16:58
@wyli wyli enabled auto-merge (squash) August 24, 2022 16:59
@wyli
Copy link
Copy Markdown
Contributor

wyli commented Aug 24, 2022

/build

@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Aug 29, 2022

/build

1 similar comment
@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Aug 29, 2022

/build

@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Aug 30, 2022

/build

@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Aug 30, 2022

Hi @wyli ,

I tried to run the packaging tests several times, but it always fails with no clear error.
Do you have any idea what happened or saw this before?

Thanks in advance.

@wyli
Copy link
Copy Markdown
Contributor

wyli commented Aug 30, 2022

looks like the something is using too much memory and the premerge is cancelled exit code 137 https://github.com/Project-MONAI/MONAI/runs/8084831205?check_suite_focus=true

@bhashemian
Copy link
Copy Markdown
Member Author

bhashemian commented Aug 30, 2022

looks like the something is using too much memory and the premerge is cancelled exit code 137 https://github.com/Project-MONAI/MONAI/runs/8084831205?check_suite_focus=true

Hi @Nic-Ma @wyli,
That's right! It seems that on this test, there is no cuCIM nor OpenSlide so they will be skipped but TiffFile is installed and it will load the image, which is big but not that big. Do you know what is the memory limit of this instance?

@wyli
Copy link
Copy Markdown
Contributor

wyli commented Aug 30, 2022

@bhashemian
Copy link
Copy Markdown
Member Author

bhashemian commented Aug 31, 2022

probably it's about 7GB https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources

I see, then that should be it. Tifffile backend needs to load the entire image into memory when extracting small patches. I'll find a way to fix this test.

@bhashemian
Copy link
Copy Markdown
Member Author

@wyli @Nic-Ma the memory overuse of tifffile backend is now fixed and the packaging test is passed.

@wyli
Copy link
Copy Markdown
Contributor

wyli commented Aug 31, 2022

/build

@wyli wyli merged commit aa12381 into Project-MONAI:dev Aug 31, 2022
@bhashemian bhashemian deleted the wsireader-tifffile branch September 1, 2022 01:14
wyli pushed a commit to wyli/MONAI that referenced this pull request Sep 1, 2022
* Implement TiffFileWSIReader

Signed-off-by: Behrooz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 💯 Complete

Development

Successfully merging this pull request may close these issues.

WSIReader does not support TiffFile backend

5 participants