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.

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

add dints model #3344

Merged
merged 94 commits into from
Nov 24, 2021
Merged

add dints model #3344

merged 94 commits into from
Nov 24, 2021

Conversation

dongyang0122
Copy link
Collaborator

@dongyang0122 dongyang0122 commented Nov 15, 2021

Signed-off-by: dongy dongy@nvidia.com

fixes #3106
core component of https://openaccess.thecvf.com/content/CVPR2021/papers/He_DiNTS_Differentiable_Neural_Network_Topology_Search_for_3D_Medical_Image_CVPR_2021_paper.pdf

Description

this is the primary PR that's needed for https://github.com/Project-MONAI/MONAI/milestone/28, the rest of the milestone could be done via the tutorial repo.

Status

Ready

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.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

Signed-off-by: dongy <dongy@nvidia.com>
@dongyang0122 dongyang0122 requested review from wyli and Nic-Ma November 15, 2021 15:21
@dongyang0122 dongyang0122 self-assigned this Nov 15, 2021
@dongyang0122 dongyang0122 changed the title add dints model [WIP] add dints model Nov 15, 2021
dongy and others added 22 commits November 16, 2021 07:51
Signed-off-by: dongy <dongy@nvidia.com>
Signed-off-by: dongy <dongy@nvidia.com>
Signed-off-by: dongy <dongy@nvidia.com>
Signed-off-by: dongy <dongy@nvidia.com>
Signed-off-by: dongy <dongy@nvidia.com>
Signed-off-by: dongy <dongy@nvidia.com>
Signed-off-by: dongy <dongy@nvidia.com>
Signed-off-by: dongy <dongy@nvidia.com>
Signed-off-by: dongy <dongy@nvidia.com>
Signed-off-by: dongy <dongy@nvidia.com>
Signed-off-by: dongy <dongy@nvidia.com>
Signed-off-by: dongy <dongy@nvidia.com>
Signed-off-by: dongy <dongy@nvidia.com>
Copy link
Contributor

@wyli wyli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, please see the comments in-line, and also there are some style checks https://deepsource.io/gh/Project-MONAI/MONAI/run/553698d2-c29b-4f8c-998a-4433e3c90521/python/ for your consideration

monai/networks/blocks/dints_block.py Outdated Show resolved Hide resolved
monai/networks/blocks/dints_block.py Outdated Show resolved Hide resolved
monai/networks/nets/dints.py Outdated Show resolved Hide resolved
monai/networks/nets/dints.py Outdated Show resolved Hide resolved
monai/networks/nets/dints.py Outdated Show resolved Hide resolved
monai/networks/nets/dints.py Show resolved Hide resolved
monai/networks/blocks/dints_block.py Outdated Show resolved Hide resolved
monai/networks/blocks/dints_block.py Outdated Show resolved Hide resolved
monai/networks/nets/dints.py Outdated Show resolved Hide resolved
@Nic-Ma
Copy link
Contributor

Nic-Ma commented Nov 24, 2021

For the Torchscript:

  1. It's saying what the issue is, something of the form var: Type isn't allowed without assigning something to var because this needs to be translated into an equivalent C++ statement. You can safely assign x to it in this case since it will be set to something else immediately in the following loop.
  2. Generated expressions like this aren't allowed so you'll have to define this with a explicit loop.
    sum(self.ops[idx.item()](x) * ops[idx.item()] * weight[idx.item()] for idx in (ops == 1).nonzero()) just becomes:
    ops_sum:float =0
    for idx in range(len(ops)):
        if ops[idx] == 1:
            ops_sum += self.ops[idx](x) * ops[idx.item()] * weight[idx.item()]

This is likely faster anyhow.

Hi @ericspod ,

Thanks very much for your help here.
I updated the code to:

ops_sum = 0
for i, o in enumerate(ops):
    if o == 1:
        ops_sum += self.ops[i](x) * o * weight[i]
return ops_sum

But still got the TorchScript error about index:

RuntimeError: 
Expected integer literal for index. ModuleList/Sequential indexing is only supported with integer literals. Enumeration is supported, e.g. 'for index, v in enumerate(self): ...':
  File "/workspace/data/medical/MONAI/monai/networks/nets/dints.py", line 132
        for i, o in enumerate(ops):
            if o == 1:
                ops_sum += self.ops[i](x) * o * weight[i]
                           ~~~~~~~~~~~ <--- HERE
        return ops_sum

I tried to change the index value but didn't solve it.. do you have any idea?

Thanks in advance.

heyufan1995 and others added 9 commits November 23, 2021 22:49
Signed-off-by: dongy <dongy@nvidia.com>
Signed-off-by: dongy <dongy@nvidia.com>
Signed-off-by: dongy <dongy@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli
Copy link
Contributor

wyli commented Nov 24, 2021

/black

@wyli
Copy link
Contributor

wyli commented Nov 24, 2021

I'm getting some compatible modules for torchscript, still testing them

wyli added 5 commits November 24, 2021 11:19
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli wyli changed the title [WIP] add dints model add dints model Nov 24, 2021
@wyli
Copy link
Contributor

wyli commented Nov 24, 2021

/build

@ericspod
Copy link
Member

ericspod commented Nov 24, 2021

@Nic-Ma,
Torchscript is finicky at times about what it will accept, it has to be compatible with C++ so it excludes many Python idioms like enumerate. You will have to loop through indices old school:

ops_sum = 0
for i in range(len(ops)):
    if ops[i] == 1:
        ops_sum += self.ops[i](x) * ops[i] * weight[i]
return ops_sum

Torchscript can understand what to do with for i in range(len(ops)) in terms of converting it to a construct like for(int i=0;i<ops.size();i++).

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli
Copy link
Contributor

wyli commented Nov 24, 2021

/build

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli
Copy link
Contributor

wyli commented Nov 24, 2021

/build

Copy link
Contributor

@wyli wyli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We added the basic APIs for DiNTS, with both 2d and 3D support.
after the architecture search, the generated network structure is compatible with torchscript JIT. We may have follow-up PRs to revise the details, but the general functionality is tested locally with the tutorials.

@wyli wyli enabled auto-merge (squash) November 24, 2021 16:15
@wyli wyli merged commit 8c90c59 into Project-MONAI:dev Nov 24, 2021
@wyli wyli mentioned this pull request Nov 24, 2021
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating tutorials for both network architecture search and operation Search
6 participants