-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Feature added: Get WSI at mpp #7574
base: dev
Are you sure you want to change the base?
Feature added: Get WSI at mpp #7574
Conversation
…izing function to Image.resize, cucim.skimage.transform.resize
0b7322e
to
1c10112
Compare
…roject-MONAI#7308) ### Description Based on the discussion topic [here](Project-MONAI#7161 (comment)), we implemented the Conjugate-Gradient algorithm for linear operator inversion, and Stein's Unbiased Risk Estimator (SURE) [1] loss for ground-truth-date free diffusion process guidance that is proposed in [2] and illustrated in the algorithm below: <img width="650" alt="Screenshot 2023-12-10 at 10 19 25 PM" src="https://github.com/Project-MONAI/MONAI/assets/8581162/97069466-cbaf-44e0-b7a7-ae9deb8fd7f2"> The Conjugate-Gradient (CG) algorithm is used to solve for the inversion of the linear operator in Line-4 in the algorithm above, where the linear operator is too large to store explicitly as a matrix (such as FFT/IFFT of an image) and invert directly. Instead, we can solve for the linear inversion iteratively as in CG. The SURE loss is applied for Line-6 above. This is a differentiable loss function that can be used to train/giude an operator (e.g. neural network), where the pseudo ground truth is available but the reference ground truth is not. For example, in the MRI reconstruction, the pseudo ground truth is the zero-filled reconstruction and the reference ground truth is the fully sampled reconstruction. The reference ground truth is not available due to the lack of fully sampled. **Reference** [1] Stein, C.M.: Estimation of the mean of a multivariate normal distribution. Annals of Statistics 1981 [[paper link](https://projecteuclid.org/journals/annals-of-statistics/volume-9/issue-6/Estimation-of-the-Mean-of-a-Multivariate-Normal-Distribution/10.1214/aos/1176345632.full)] [2] B. Ozturkler et al. SMRD: SURE-based Robust MRI Reconstruction with Diffusion Models. MICCAI 2023 [[paper link](https://arxiv.org/pdf/2310.01799.pdf)] ### Types of changes <!--- Put an `x` in all the boxes that apply, and remove the not applicable items --> - [x] 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). - [x] New tests added to cover the changes. - [x] Integration tests passed locally by running `./runtests.sh -f -u --net --coverage`. - [x] Quick tests passed locally by running `./runtests.sh --quick --unittests --disttests`. - [x] In-line docstrings updated. - [x] Documentation updated, tested `make html` command in the `docs/` folder. --------- Signed-off-by: chaoliu <[email protected]> Signed-off-by: cxlcl <[email protected]> Signed-off-by: chaoliu <[email protected]> Signed-off-by: YunLiu <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: YunLiu <[email protected]> Co-authored-by: Eric Kerfoot <[email protected]> Signed-off-by: Nikolas Schmitz <[email protected]>
Signed-off-by: Nikolas Schmitz <[email protected]>
for more information, see https://pre-commit.ci Signed-off-by: Nikolas Schmitz <[email protected]>
Signed-off-by: Nikolas Schmitz <[email protected]>
Signed-off-by: monai-bot <[email protected]> Signed-off-by: monai-bot <[email protected]> Signed-off-by: Nikolas Schmitz <[email protected]>
Signed-off-by: Nikolas Schmitz <[email protected]>
1c10112
to
ae704f3
Compare
Signed-off-by: Nikolas Schmitz <[email protected]>
…ject-MONAI#7569) Fixes Project-MONAI#7451 ### Description Reduces the length of error messages and error messages being propagated twice. This helps debug better when long `ConfigComponent`s are being instantiated. Refer to issue Project-MONAI#7451 for more details ### Types of changes <!--- Put an `x` in all the boxes that apply, and remove the not applicable items --> - [x] 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. - [ ] Integration tests passed locally by running `./runtests.sh -f -u --net --coverage`. - [x] 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. Signed-off-by: Suraj Pai <[email protected]> Co-authored-by: Eric Kerfoot <[email protected]>
Fixes Project-MONAI#2872 ### Description Implementation of mixup, cutmix and cutout as described in the original papers. Current implementation support both, the dictionary-based batches and tuples of tensors. ### Types of changes <!--- Put an `x` in all the boxes that apply, and remove the not applicable items --> - [x] 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). - [x] New tests added to cover the changes. - [ ] Integration tests passed locally by running `./runtests.sh -f -u --net --coverage`. - [x] Quick tests passed locally by running `./runtests.sh --quick --unittests --disttests`. - [x] In-line docstrings updated. - [x] Documentation updated, tested `make html` command in the `docs/` folder. --------- Signed-off-by: Juan Pablo de la Cruz Gutiérrez <[email protected]> Signed-off-by: monai-bot <[email protected]> Signed-off-by: elitap <[email protected]> Signed-off-by: Felix Schnabel <[email protected]> Signed-off-by: YanxuanLiu <[email protected]> Signed-off-by: ytl0623 <[email protected]> Signed-off-by: Dženan Zukić <[email protected]> Signed-off-by: KumoLiu <[email protected]> Signed-off-by: YunLiu <[email protected]> Signed-off-by: Ishan Dutta <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: kaibo <[email protected]> Signed-off-by: heyufan1995 <[email protected]> Signed-off-by: binliu <[email protected]> Signed-off-by: axel.vlaminck <[email protected]> Signed-off-by: Ibrahim Hadzic <[email protected]> Signed-off-by: Behrooz <[email protected]> Signed-off-by: Timothy Baker <[email protected]> Signed-off-by: Mathijs de Boer <[email protected]> Signed-off-by: Fabian Klopfer <[email protected]> Signed-off-by: Lucas Robinet <[email protected]> Signed-off-by: Lucas Robinet <[email protected]> Signed-off-by: chaoliu <[email protected]> Signed-off-by: cxlcl <[email protected]> Signed-off-by: chaoliu <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: monai-bot <[email protected]> Co-authored-by: elitap <[email protected]> Co-authored-by: Felix Schnabel <[email protected]> Co-authored-by: YanxuanLiu <[email protected]> Co-authored-by: ytl0623 <[email protected]> Co-authored-by: Dženan Zukić <[email protected]> Co-authored-by: Eric Kerfoot <[email protected]> Co-authored-by: YunLiu <[email protected]> Co-authored-by: Ishan Dutta <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Kaibo Tang <[email protected]> Co-authored-by: Yufan He <[email protected]> Co-authored-by: binliunls <[email protected]> Co-authored-by: Ben Murray <[email protected]> Co-authored-by: axel.vlaminck <[email protected]> Co-authored-by: Mingxin Zheng <[email protected]> Co-authored-by: Ibrahim Hadzic <[email protected]> Co-authored-by: Dr. Behrooz Hashemian <[email protected]> Co-authored-by: Timothy J. Baker <[email protected]> Co-authored-by: Mathijs de Boer <[email protected]> Co-authored-by: Mathijs de Boer <[email protected]> Co-authored-by: Fabian Klopfer <[email protected]> Co-authored-by: Yiheng Wang <[email protected]> Co-authored-by: Lucas Robinet <[email protected]> Co-authored-by: Lucas Robinet <[email protected]> Co-authored-by: cxlcl <[email protected]>
a65ef4a
to
3264079
Compare
Signed-off-by: Dr. Behrooz Hashemian <[email protected]>
@NikolasSchmitz, I resolved the conflicts and updated the PR. Let's focus on the functionality and make this PR ready for reviewing. We can take care of any other issue once the PR is ready and please feel free to reach out to me or the working group if you still have any questions. Thanks for taking on this feature. |
Thank you @drbeh ! I now marked it as ready for review. |
@drbeh would you be able to review this? I think you're best qualified here. I made some minor comments about print and code duplication but otherwise it seems fine to me without having tested it. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the effort @NikolasSchmitz. A welcome addition! I agree with @ericspod about the refactoring to remove duplication - that would be great.
Thank you for the feedback, @JHancox . Unfortunately, I couldn't join the meeting because I was on my way home from ICLR. |
… reduce redundancy; for get_mpp of TiffFileWSIReader: added check to prevent division by zero error.
I factored out the actual resizing of the image into _resize_to_mpp_res() and kept the tolerance checks in get_wsi_at_mpp(). I had some problems with specific tiff files and the TiffFile backend. Some files had zero as X/Y Resolution, which led to a division by zero error in the function get_mpp (added a check for that in the new commit). |
There are lines added/removed to two other files that don't need to be changed in this PR, tests/test_regularization.py is causing a conflict for example. I would remove those and we can use black here through Github to resolve formatting issues if we need to. |
Thanks for the info. Interesting though, haven't noticed that these files were changed. Perhaps through one of the coding style checks. |
Signed-off-by: Nikolas Schmitz <[email protected]>
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
There's a few changes to do but it looks good in general. We do need tests for these new methods as well so please look at the existing WSI tests to see what needs to be added to cover new functionality. The flake8 and DCO fixes can be done at the end. Thanks! |
…ces to BaseWSIReader; Edited docstrings
for more information, see https://pre-commit.ci
mpp_list = [self.get_mpp(wsi, lvl) for lvl in range(wsi.resolutions["level_count"])] | ||
closest_lvl = self._find_closest_level("mpp", mpp, mpp_list, 0, 5) | ||
|
||
within_tolerance, closest_level_is_bigger = super()._compute_mpp_tolerances(closest_lvl, mpp_list, mpp, atol, rtol) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
within_tolerance, closest_level_is_bigger = super()._compute_mpp_tolerances(closest_lvl, mpp_list, mpp, atol, rtol) | |
within_tolerance, closest_level_is_bigger = self._compute_mpp_tolerances(closest_lvl, mpp_list, mpp, atol, rtol) |
If you're not overriding the method in this class you can refer to it through self
as it will be inherited.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same in other places below.
Fixes: #4980
Description
In this pull request, the feature to retrieve a whole slide image at a given mpp (microns per pixel) resolution was implemented for every WSIReader class in the function
get_wsi_at_mpp
.While the implementations in the
OpenslideWSIReader
andCuCIMWSIReader
classes were tested thoroughly, I could not find a suitable TIFF file for testing with theTiffFileWSIReader
class.For resizing, I have used
PIL.Image.resize
for Openslide and TiffFile, andcucim.sklearn.transform.resize
for CuCIM. Originally, I usedcv2.resize
, but since the package isn't listed in requirements-dev.txt, I explored alternative solutions."Types of changes
./runtests.sh -f -u --net --coverage
../runtests.sh --quick --unittests --disttests
.make html
command in thedocs/
folder.Please let me know what you think and how I can improve this feature.
Best
Niko
(Last PR was closed, because I changed the branch name to include the ticket id)