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

Add compile support in SupervisedTrainer and SupervisedEvaluator #7375

Merged
merged 25 commits into from
Jan 19, 2024

Conversation

KumoLiu
Copy link
Contributor

@KumoLiu KumoLiu commented Jan 8, 2024

Fixes # .

Description

Add compile support in SupervisedTrainer and SupervisedEvaluator.
Convert to torch.Tensor internally.

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.

Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
monai/engines/evaluator.py Outdated Show resolved Hide resolved
monai/engines/evaluator.py Outdated Show resolved Hide resolved
KumoLiu and others added 6 commits January 9, 2024 17:22
Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
@KumoLiu
Copy link
Contributor Author

KumoLiu commented Jan 16, 2024

From the suggestion here, I tried torch-nightly. I can run the pipeline without converting MetaTensor to tensor after updating traceable_tensor_subclasses.

But as mentioned here, this feature doesn't really work and will not preserve the meta information.
https://github.com/pytorch/pytorch/blob/13473df0d770e67f92cb4bb1b728f3746f160b7c/torch/_dynamo/config.py#L101-L112

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

I'm not sure that the support in Pytorch will be stable soon to be compatible with MetaTensor. If we want to integrate this we'll have to put a warning into the docstring stating that MetaTensor will be cast away. We could consider casting back to MetaTensor after the forward pass and copying the metadata from the original but this is different behaviour again that may not make things better.

KumoLiu and others added 4 commits January 17, 2024 15:02
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
@KumoLiu
Copy link
Contributor Author

KumoLiu commented Jan 17, 2024

I'm not sure that the support in Pytorch will be stable soon to be compatible with MetaTensor. If we want to integrate this we'll have to put a warning into the docstring stating that MetaTensor will be cast away. We could consider casting back to MetaTensor after the forward pass and copying the metadata from the original but this is different behaviour again that may not make things better.

Thanks, Eric for your suggestions, I have added a warning and also mentioned it in the docstring, I cast back to the MetaTensor after the forward pass. Could you please help review it again? Then I will add some tests.

Signed-off-by: YunLiu <55491388+KumoLiu@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 to me now, I had just a minor suggestion for the comment that should be in the Evaluator and Trainer classes.

KumoLiu and others added 5 commits January 18, 2024 12:48
Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
@KumoLiu KumoLiu marked this pull request as ready for review January 18, 2024 04:51
@KumoLiu
Copy link
Contributor Author

KumoLiu commented Jan 18, 2024

/build

@KumoLiu
Copy link
Contributor Author

KumoLiu commented Jan 19, 2024

/build

@KumoLiu KumoLiu enabled auto-merge (squash) January 19, 2024 02:41
@KumoLiu KumoLiu requested a review from Nic-Ma January 19, 2024 02:41
@KumoLiu
Copy link
Contributor Author

KumoLiu commented Jan 19, 2024

Known issue list here as a reminder. pytorch/pytorch#117026

@KumoLiu
Copy link
Contributor Author

KumoLiu commented Jan 19, 2024

/build

@KumoLiu KumoLiu merged commit facf176 into Project-MONAI:dev Jan 19, 2024
28 checks passed
@KumoLiu KumoLiu deleted the torch2-1 branch January 19, 2024 08:41
marksgraham pushed a commit to marksgraham/MONAI that referenced this pull request Jan 30, 2024
Project-MONAI#7375)

Fixes # .

### Description

Add `compile` support in `SupervisedTrainer` and `SupervisedEvaluator`.
Convert to `torch.Tensor` internally.

### 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`.
- [ ] 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: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Signed-off-by: Mark Graham <markgraham539@gmail.com>
juampatronics pushed a commit to juampatronics/MONAI that referenced this pull request Mar 25, 2024
Project-MONAI#7375)

Fixes # .

### Description

Add `compile` support in `SupervisedTrainer` and `SupervisedEvaluator`.
Convert to `torch.Tensor` internally.

### 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`.
- [ ] 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: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Signed-off-by: Juan Pablo de la Cruz Gutiérrez <juampatronics@gmail.com>
Yu0610 pushed a commit to Yu0610/MONAI that referenced this pull request Apr 11, 2024
Project-MONAI#7375)

Fixes # .

### Description

Add `compile` support in `SupervisedTrainer` and `SupervisedEvaluator`.
Convert to `torch.Tensor` internally.

### 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`.
- [ ] 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: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@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.

2 participants