Fix AutoencoderKlMaisi forcing CUDA transfer on CPU inputs#8736
Fix AutoencoderKlMaisi forcing CUDA transfer on CPU inputs#8736ericspod merged 6 commits intoProject-MONAI:devfrom
Conversation
Signed-off-by: ytl0623 <david89062388@gmail.com>
📝 WalkthroughWalkthroughThe Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
monai/apps/generation/maisi/networks/autoencoderkl_maisi.py (1)
217-232:⚠️ Potential issue | 🟡 MinorFix is correct and improves on the device-handling pattern.
Unconditionally moves tensors to CPU for concatenation (line 219), then conditionally returns to original device only if non-CPU (line 230-231). Unlike
_cat_inputswhich hardcodes "cuda", this approach works for any device type.However, existing tests use input shape (1, 1, 32, 32, 32) with max size 32, which falls below the 500-element threshold (line 214) and skips this large-tensor concatenation branch entirely. A test with max(size) ≥ 500 is needed to exercise and prevent regression in lines 217-231.
…ONAI#8736) Fixes Project-MONAI#8735 ### Description This PR fixes a bug in `AutoencoderKlMaisi` where the model would force a transfer to `cuda` even if the input tensors and the model were placed on the `CPU`. ### 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: ytl0623 <david89062388@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
…ONAI#8736) Fixes Project-MONAI#8735 ### Description This PR fixes a bug in `AutoencoderKlMaisi` where the model would force a transfer to `cuda` even if the input tensors and the model were placed on the `CPU`. ### 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: ytl0623 <david89062388@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Fixes #8735
Description
This PR fixes a bug in
AutoencoderKlMaisiwhere the model would force a transfer tocudaeven if the input tensors and the model were placed on theCPU.Types of changes
./runtests.sh -f -u --net --coverage../runtests.sh --quick --unittests --disttests.make htmlcommand in thedocs/folder.