Inconsistent usage of the linear layer in the SABlock #7992

Activity

1. ericspod commented on Aug 5, 2024

   

   ericspod

   on Aug 5, 2024

   Member

   More actions

   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.
2. KumoLiu

   added a commit that references this issue on Aug 6, 2024

   fix Project-MONAI#7992

   ...

   This issue will close once commit 0337d45 is merged into the 'dev' branch.

   0337d45
3. KumoLiu commented on Aug 6, 2024

   

   KumoLiu

   on Aug 6, 2024

   Last edited by KumoLiu

   ContributorAuthor

   More actions

   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!

   import torch.nn as nn
   import torch
   from monai.utils import set_determinism
   
   set_determinism(0)
   
   hidden_input_size = 360
   inner_dim = 360
   qkv_bias = True
   qkv = nn.Linear(hidden_input_size, inner_dim * 3, bias=qkv_bias)
   
   
   q = nn.Linear(hidden_input_size, inner_dim, bias=qkv_bias)
   k = nn.Linear(hidden_input_size, inner_dim, bias=qkv_bias)
   v = nn.Linear(hidden_input_size, inner_dim, bias=qkv_bias)
   
   input = torch.rand(2, 512, 360)
   qkv_out = qkv(input)
   q_out = q(input)
   k_out = k(input)
   v_out = v(input)
   
   print((qkv_out - torch.cat([q_out, k_out, v_out], dim=-1)).abs().sum().item()) --> 410444.28125
   
4. KumoLiu

   closed this as completedin #7996on Aug 9, 2024
5. KumoLiu

   closed this as completedin 069519don Aug 9, 2024
6. KumoLiu

   added a commit that references this issue on Aug 9, 2024

   Add include_fc and `use_combined_linear` argument in the `SABlock` (P…

   ...

   This issue will close once commit 069519d is merged into the 'dev' branch.

   069519d
7. rcremese

   added a commit that references this issue on Sep 2, 2024

   Add include_fc and `use_combined_linear` argument in the `SABlock` (P…

   ...

   This issue will close once commit 0d7744f is merged into the 'dev' branch.

   0d7744f