Project-MONAI / MONAI

AI Toolkit for Healthcare Imaging
https://monai.io/
Apache License 2.0
5.87k stars 1.09k forks source link

Add vista network #7987

Closed heyufan1995 closed 3 months ago

heyufan1995 commented 3 months ago

Fixes # .

Description

Add VISTA3D model architecture to MONAI core

Types of changes

ericspod commented 3 months ago

Hi @heyufan1995 Thanks for the contribution from the VISTA team! I've added a few comments on style and such to try improving readability and number of lines. We will also need unit tests for anything added. You can also run the auto formatting fixes with runtests.sh.

yiheng-wang-nv commented 3 months ago

HI @heyufan1995 , I suggest we split this PR into three small PRs:

  1. add util functions
  2. add SegResNetDS2
  3. add vista3d

To merge int MONAI, there are some basic requirements:

  1. correct code format
  2. enough unit tests
  3. enough doc strings

It would be more efficient to handle each parts separately

yiheng-wang-nv commented 3 months ago

HI @heyufan1995 , I suggest we split this PR into three small PRs:

  1. add util functions
  2. add SegResNetDS2
  3. add vista3d

To merge int MONAI, there are some basic requirements:

  1. correct code format
  2. enough unit tests
  3. enough doc strings

It would be more efficient to handle each parts separately

for utils functions, I copy the content and submit a draft PR: https://github.com/Project-MONAI/MONAI/pull/7999 Will address issues there

yiheng-wang-nv commented 3 months ago

other discussed topics:

  1. TwoWayTransformer, TwoWayAttentionBlock, PositionEmbeddingRandom and other blocks may move to other files.
  2. SABlock, may need to adjust to use different linear layers for q, k, v respectively
KumoLiu commented 3 months ago

FYI, SABlock will be fixed in this PR:https://github.com/Project-MONAI/MONAI/pull/7996. @yiheng-wang-nv @heyufan1995

yiheng-wang-nv commented 3 months ago

FYI, SABlock will be fixed in this PR:#7996. @yiheng-wang-nv @heyufan1995

HI @KumoLiu @heyufan1995 , can I double confirm if SABlock can replace Attention here? The module structure can be the same, but for the forward function,

the input for SABlock is a single tensor:

    def forward(self, x):
        """
        Args:
            x (torch.Tensor): input tensor. B x (s_dim_1 * ... * s_dim_n) x C

        Return:
            torch.Tensor: B x (s_dim_1 * ... * s_dim_n) x C
        """

the attention module vista3d needs three input:

    def forward(self, q: Tensor, k: Tensor, v: Tensor) -> Tensor:
        # Input projections
        q = self.q_proj(q)
        k = self.k_proj(k)
        v = self.v_proj(v)
KumoLiu commented 3 months ago

Hi @heyufan1995 and @yiheng-wang-nv, please help take a look at the ci issue. https://github.com/Project-MONAI/MONAI/actions/runs/10374743344/job/28722878047?pr=7987#step:7:392 https://github.com/Project-MONAI/MONAI/actions/runs/10374743344/job/28722877791?pr=7987 Thanks.

KumoLiu commented 3 months ago

/build