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

Refactoring ConvModule by removing norm_cfg #3816

Merged
merged 27 commits into from
Aug 19, 2024

Conversation

sungchul2
Copy link
Contributor

@sungchul2 sungchul2 commented Aug 8, 2024

Summary

This PR includes:

  • Update build_norm_layer

    • This function can handle various format of normalization:

      given format returned format
      tuple[norm_name, norm_layer] generated by build_norm_layer before tuple[norm_name, nn.Module] returned as is
      nn.Module predefined module ("", nn.Module) returned with empty name string
      nn.BatchNorm2d type is only given (norm_name, nn.Module) built and returned with default settings and given num_features
      partial(nn.BatchNorm2d, eps=1) partial for norm layer is given (norm_name, nn.Module) built and returned with custom settings and given num_features
      partial(build_norm_layer(nn.BatchNorm2d, requires_grad=True)) partial of build_norm_layer for arguments of build_norm_layer (norm_name, nn.Module) built and returned with custom settings and given num_features
  • Replace norm_cfg with normalization

  • Restore build_activation_layer

  • Update ConvModule to use pre-instantiated normalization module and activation module

  • Update activation docstring

How to test

Checklist

  • I have added unit tests to cover my changes.​
  • I have added integration tests to cover my changes.​
  • I have ran e2e tests and there is no issues.
  • I have added the description of my changes into CHANGELOG in my target branch (e.g., CHANGELOG in develop).​
  • I have updated the documentation in my target branch accordingly (e.g., documentation in develop).
  • I have linked related issues.

License

  • I submit my code changes under the same Apache License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below).
# Copyright (C) 2024 Intel Corporation
# SPDX-License-Identifier: Apache-2.0

@github-actions github-actions bot added TEST Any changes in tests DOC Improvements or additions to documentation OTX 2.0 labels Aug 8, 2024
@sungchul2 sungchul2 force-pushed the remove-norm_cfg branch 2 times, most recently from aed5b08 to b77cc8c Compare August 13, 2024 02:00
@sungchul2 sungchul2 enabled auto-merge August 15, 2024 14:57
@sungchul2
Copy link
Contributor Author

sungchul2 commented Aug 15, 2024

@sooahleex @harimkang I saw the performance drop for deit_tiny and efficientnet_v2 on hlabel classification as below.
I used only medium set (hlabel_CUB_medium), but classification has been using deterministic seed and its performance was maintained.

commit deit_tiny efficientnet_v2
1a8e10e 0.93 0.97
2ecaac1 0.87 0.39

If you already know this, feel free to ignore it :)

@sungchul2 sungchul2 requested a review from harimkang August 15, 2024 15:06
@sungchul2 sungchul2 disabled auto-merge August 15, 2024 15:24
harimkang
harimkang previously approved these changes Aug 16, 2024
harimkang
harimkang previously approved these changes Aug 16, 2024
Copy link
Contributor

@eunwoosh eunwoosh left a comment

Choose a reason for hiding this comment

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

Thanks for your lots of work :) LGTM. I left some minor comments. Please check it's worth to apply later.

src/otx/algo/modules/activation.py Show resolved Hide resolved
src/otx/algo/modules/conv_module.py Show resolved Hide resolved
src/otx/algo/modules/conv_module.py Show resolved Hide resolved
src/otx/algo/modules/conv_module.py Show resolved Hide resolved
@sungchul2 sungchul2 added this pull request to the merge queue Aug 19, 2024
Merged via the queue into openvinotoolkit:develop with commit f50e821 Aug 19, 2024
18 checks passed
@sungchul2 sungchul2 deleted the remove-norm_cfg branch August 19, 2024 03:28
@sungchul2 sungchul2 mentioned this pull request Aug 21, 2024
8 tasks
github-merge-queue bot pushed a commit that referenced this pull request Aug 21, 2024
* Reflect comments in #3816

* Enable nn.Module standalone

* Update
sooahleex pushed a commit to sooahleex/training_extensions that referenced this pull request Aug 26, 2024
* Reflect comments in openvinotoolkit#3816

* Enable nn.Module standalone

* Update
sooahleex pushed a commit to sooahleex/training_extensions that referenced this pull request Aug 26, 2024
* Reflect comments in openvinotoolkit#3816

* Enable nn.Module standalone

* Update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DOC Improvements or additions to documentation TEST Any changes in tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants