-
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
Inconsistent usage of the linear layer in the SABlock #7992
Comments
Perhaps we change the current implementation to match this instead? I don't know if it's significant but making this an option will be problematic with Torchscript which doesn't like class variables being optional. |
KumoLiu
added a commit
to KumoLiu/MONAI
that referenced
this issue
Aug 6, 2024
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
I did a quick test, looks the outcome shows significantly different. In this PR, I utilized identity to get around the torchscript conversion issue. Could you please help review it, thanks!
|
7 tasks
rcremese
pushed a commit
to rcremese/MONAI
that referenced
this issue
Sep 2, 2024
…roject-MONAI#7996) Fixes Project-MONAI#7991 Fixes Project-MONAI#7992 ### Description Add `include_fc` and `use_combined_linear` argument in the `SABlock`. ### 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: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There are three linear in the original generative repo which now is combined within one linear layer. We need add an argument to make it consistent with before.
https://github.com/Project-MONAI/GenerativeModels/blob/main/generative/networks/nets/diffusion_model_unet.py#L379-L381
MONAI/monai/networks/blocks/selfattention.py
Line 89 in 56ee32e
The text was updated successfully, but these errors were encountered: