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

Stein's Unbiased Risk Estimator (SURE) loss and Conjugate Gradient #7308

Merged
merged 41 commits into from
Mar 22, 2024

Conversation

cxlcl
Copy link
Contributor

@cxlcl cxlcl commented Dec 11, 2023

Description

Based on the discussion topic here, 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:

Screenshot 2023-12-10 at 10 19 25 PM

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]
[2] B. Ozturkler et al. SMRD: SURE-based Robust MRI Reconstruction with Diffusion Models. MICCAI 2023
[paper link]

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.
  • 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.

cxlcl and others added 4 commits December 10, 2023 20:18
Signed-off-by: chaoliu <chaoliu@nvidia.com>
Signed-off-by: chaoliu <chaoliu@nvidia.com>
Signed-off-by: chaoliu <chaoliu@nvidia.com>
@ericspod
Copy link
Member

@marksgraham you may want to coordinate here with other diffusion work being integrated into core.

@Project-MONAI Project-MONAI deleted a comment from ericspod Dec 11, 2023
@KumoLiu
Copy link
Contributor

KumoLiu commented Dec 11, 2023

Sorry @ericspod, I accidentally deleted your comment, I just wanted to delete the pending one from me which duplicate with yours.

Copy link
Contributor

@KumoLiu KumoLiu left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, would be great add some user guide in the tutorial repo perhaps after all components added. Could you please help fix the CI error?

monai/networks/layers/conjugate_gradient.py Outdated Show resolved Hide resolved
@marksgraham
Copy link
Contributor

@marksgraham you may want to coordinate here with other diffusion work being integrated into core.

Thanks @ericspod. This PR looks complementary - @cxlcl let me know if you want to discuss when you get onto adding the generation step work mentioned in your original issue. MONAI Generative is currently being ported into core so we need to make sure these additions work together :)

@cxlcl
Copy link
Contributor Author

cxlcl commented Dec 11, 2023

@marksgraham you may want to coordinate here with other diffusion work being integrated into core.

Thanks @ericspod. This PR looks complementary - @cxlcl let me know if you want to discuss when you get onto adding the generation step work mentioned in your original issue. MONAI Generative is currently being ported into core so we need to make sure these additions work together :)

@marksgraham For the generation step, I was planning to use the two components in this PR (CG and SURE loss) to replace the ones in the original implementation in Reference [2]. This is done locally and I will upload it to the research repo of MONA as an example of how to use CG and SURE during the sampling stage. Will let you know once it's done. So that we may start from there.

Signed-off-by: cxlcl <chaoliucxl@gmail.com>
@cxlcl
Copy link
Contributor Author

cxlcl commented Dec 11, 2023

Thanks for the PR, would be great add some user guide in the tutorial repo perhaps after all components added. Could you please help fix the CI error?

Will work on that. Thanks for the suggestion on making a suer guide. Is there a guidance on making a tutorial or exemplar tutorial I can follow?

@KumoLiu
Copy link
Contributor

KumoLiu commented Dec 12, 2023

Is there a guidance on making a tutorial or exemplar tutorial I can follow?

Here is a contribution guide for tutorial repo.
https://github.com/Project-MONAI/tutorials/blob/main/CONTRIBUTING.md

@cxlcl
Copy link
Contributor Author

cxlcl commented Jan 19, 2024

Is there a guidance on making a tutorial or exemplar tutorial I can follow?

Here is a contribution guide for tutorial repo. https://github.com/Project-MONAI/tutorials/blob/main/CONTRIBUTING.md

One quick question related to the data used for the tutorial: can we include it in the tutorial itself, or shall we include it anywhere else (such as a link) and then use wget to download it?

@ericspod
Copy link
Member

If you can include the data directly in the notebook (ie. in a cell) you can do that, otherwise please host it elsewhere such that it can be downloaded through the browser or with a tool like gdown that's readily available.

cxlcl and others added 10 commits January 28, 2024 16:29
Signed-off-by: chaoliu <chaoliucxl@gmail.com>
Signed-off-by: chaoliu <chaoliucxl@gmail.com>
Signed-off-by: chaoliu <chaoliucxl@gmail.com>
Signed-off-by: chaoliu <chaoliucxl@gmail.com>
Signed-off-by: chaoliu <chaoliucxl@gmail.com>
Signed-off-by: chaoliu <chaoliucxl@gmail.com>
@cxlcl
Copy link
Contributor Author

cxlcl commented Jan 29, 2024

Hi, maybe I didn't quite get how the script ./runtest.sh decide if a the code format is ready to go, and reformatted it if needed. But after I tried to ran ./runtest.sh --autofix after getting this CI error, I got this CI error.

It turns out that the ./runtest.sh --autofix did the changes similar to the following to multiple files:

before (in: monai/data/decathlon_datalist.py)

@overload
def _compute_path(base_dir: PathLike, element: PathLike, check_path: bool = False) -> str:
    ...

after

@overload
def _compute_path(base_dir: PathLike, element: PathLike, check_path: bool = False) -> str: ...

These changes are applied to multiple files and lead to the following error in the CI test [link to the error message]

/home/runner/work/MONAI/MONAI/monai/data/decathlon_datalist.py:27:1: E704 multiple statements on one line (def)
/home/runner/work/MONAI/MONAI/monai/data/decathlon_datalist.py:31:1: E704 multiple statements on one line (def)
/home/runner/work/MONAI/MONAI/monai/utils/dist.py:53:1: E704 multiple statements on one line (def)
/home/runner/work/MONAI/MONAI/monai/utils/dist.py:57:1: E704 multiple statements on one line (def)
/home/runner/work/MONAI/MONAI/monai/utils/dist.py:61:1: E704 multiple statements on one line (def)
/home/runner/work/MONAI/MONAI/monai/utils/misc.py:106:1: E704 multiple statements on one line (def)
/home/runner/work/MONAI/MONAI/monai/utils/misc.py:110:1: E704 multiple statements on one line (def)
7     E704 multiple statements on one line (def)
7
Check failed!
Please run auto style fixes: ./runtests.sh --autofix
Error: Process completed with exit code 1.

As suggested in the guideline, I've tried to run ./runtests.sh --autofix locally before pushing. But after that, when I run ./runtests.sh --codeformat, I got the same error above.

Maybe I'm missing something, like the configuration for autofix?

@KumoLiu
Copy link
Contributor

KumoLiu commented Jan 29, 2024

Hi @cxlcl, it looks like it was introduced by the new release from black.
psf/black#4173
We should ignore E704. cc @ericspod

@cxlcl
Copy link
Contributor Author

cxlcl commented Jan 30, 2024

Hi @cxlcl, it looks like it was introduced by the new release from black. psf/black#4173 We should ignore E704. cc @ericspod

Thanks for the reply. Is there anything I can do from my end to pass the CI test to ignore E704? I guess it is a server-side modification for testing.

@KumoLiu
Copy link
Contributor

KumoLiu commented Jan 30, 2024

Hi @cxlcl, no.
I have created a PR to fix it. #7422
You can directly pull the latest change, thanks.

cxlcl and others added 7 commits February 24, 2024 09:23
Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Signed-off-by: cxlcl <chaoliucxl@gmail.com>
Signed-off-by: chaoliu <chaoliucxl@gmail.com>
Signed-off-by: chaoliu <chaoliucxl@gmail.com>
@cxlcl
Copy link
Contributor Author

cxlcl commented Feb 24, 2024

Hi @cxlcl I went through this and the tutorial with a number of comments. It looks good in general though there's a few things to address. Please do still coordinate with @marksgraham if there's anything to align with our other diffusion work. Thanks!

Hi @ericspod , thanks a lot for the comments and suggestions. I did some modifications based on the suggestions in the new commit. Please let me know if there's any modification needed. Thanks!

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 now. There's a minor PEP8 complaint but otherwise it's fine.

Copy link
Contributor

@KumoLiu KumoLiu left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, thanks!

docs/source/losses.rst Outdated Show resolved Hide resolved
monai/losses/sure_loss.py Outdated Show resolved Hide resolved
monai/losses/sure_loss.py Outdated Show resolved Hide resolved
monai/losses/sure_loss.py Outdated Show resolved Hide resolved
tests/test_conjugate_gradient.py Outdated Show resolved Hide resolved
tests/test_conjugate_gradient.py Outdated Show resolved Hide resolved
tests/test_sure_loss.py Outdated Show resolved Hide resolved
tests/test_sure_loss.py Outdated Show resolved Hide resolved
tests/test_sure_loss.py Outdated Show resolved Hide resolved
cxlcl and others added 12 commits March 7, 2024 18:58
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: cxlcl <chaoliucxl@gmail.com>
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: cxlcl <chaoliucxl@gmail.com>
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: cxlcl <chaoliucxl@gmail.com>
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: cxlcl <chaoliucxl@gmail.com>
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: cxlcl <chaoliucxl@gmail.com>
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: cxlcl <chaoliucxl@gmail.com>
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: cxlcl <chaoliucxl@gmail.com>
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: cxlcl <chaoliucxl@gmail.com>
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: cxlcl <chaoliucxl@gmail.com>
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: cxlcl <chaoliucxl@gmail.com>
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
@KumoLiu
Copy link
Contributor

KumoLiu commented Mar 22, 2024

/build

Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
@KumoLiu
Copy link
Contributor

KumoLiu commented Mar 22, 2024

/build

@KumoLiu KumoLiu merged commit c3a7383 into Project-MONAI:dev Mar 22, 2024
28 checks passed
juampatronics pushed a commit to juampatronics/MONAI that referenced this pull request Mar 25, 2024
…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 <chaoliu@nvidia.com>
Signed-off-by: cxlcl <chaoliucxl@gmail.com>
Signed-off-by: chaoliu <chaoliucxl@gmail.com>
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Signed-off-by: Juan Pablo de la Cruz Gutiérrez <juampatronics@gmail.com>
NikolasSchmitz pushed a commit to NikolasSchmitz/MONAI that referenced this pull request Mar 27, 2024
…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 <chaoliu@nvidia.com>
Signed-off-by: cxlcl <chaoliucxl@gmail.com>
Signed-off-by: chaoliu <chaoliucxl@gmail.com>
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Signed-off-by: Nikolas Schmitz <nikolas.schmitz@rwth-aachen.de>
Yu0610 pushed a commit to Yu0610/MONAI that referenced this pull request Apr 11, 2024
…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 <chaoliu@nvidia.com>
Signed-off-by: cxlcl <chaoliucxl@gmail.com>
Signed-off-by: chaoliu <chaoliucxl@gmail.com>
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Signed-off-by: Yu0610 <612410030@alum.ccu.edu.tw>
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.

None yet

4 participants