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.

Sign up for GitHub

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

Jump to bottom

Add compile support in SupervisedTrainer and SupervisedEvaluator #7375

Merged
merged 25 commits into from
Jan 19, 2024
Merged

Add compile support in SupervisedTrainer and SupervisedEvaluator #7375

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.

ericspod
ericspod reviewed Jan 9, 2024
View reviewed changes
monai/engines/evaluator.py Outdated Show resolved Hide resolved
ericspod
ericspod reviewed Jan 9, 2024
View reviewed changes
monai/engines/evaluator.py Outdated Show resolved Hide resolved
KumoLiu and others added 6 commits January 9, 2024 17:22
@Nic-Ma Nic-Ma mentioned this pull request Jan 15, 2024
Closed
@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

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

@KumoLiu
fix flake8
c9b8115 
Signed-off-by: YunLiu <[email protected]>
ericspod
ericspod approved these changes Jan 17, 2024
View reviewed changes
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 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
@KumoLiu @ericspod
@pre-commit-ci @marksgraham
Add compile support in SupervisedTrainer and SupervisedEvaluator (
ef912c8 
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 <[email protected]>
Co-authored-by: Eric Kerfoot <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Signed-off-by: Mark Graham <[email protected]>
juampatronics pushed a commit to juampatronics/MONAI that referenced this pull request Mar 25, 2024
@KumoLiu @ericspod
@pre-commit-ci @juampatronics
Add compile support in SupervisedTrainer and SupervisedEvaluator (
aef4b57 
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 <[email protected]>
Co-authored-by: Eric Kerfoot <[email protected]>
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 <[email protected]>
Yu0610 pushed a commit to Yu0610/MONAI that referenced this pull request Apr 11, 2024
@KumoLiu @ericspod
@pre-commit-ci @Yu0610
Add compile support in SupervisedTrainer and SupervisedEvaluator (
80734c8 
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 <[email protected]>
Co-authored-by: Eric Kerfoot <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Signed-off-by: Yu0610 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericspod ericspod ericspod approved these changes

@Nic-Ma Nic-Ma Awaiting requested review from Nic-Ma

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KumoLiu @ericspod