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

Reimplementation of ViT for classification #3719

Merged
merged 11 commits into from
Jul 12, 2024

Conversation

wonjuleee
Copy link
Contributor

@wonjuleee wonjuleee commented Jul 9, 2024

Summary

  • Replacement of ViT implementation with Timm
  • Updated pretrained weight provided by Timm
  • Results on multi-class classification (2.2%/0.6% accuracy gain on medium/large datasets with 25% faster e2e time)
develop   test export optimize train/e2e
medium 0 0.75762     100.726
  1 0.77561     111.267
  2 0.78108     106.442
  3 0.76231     94.7122
  4 0.78655     180.741
  avg 0.772635     118.7775
large 0 0.8445     355.415
  1 0.85083     465.393
  2 0.85433     427.786
  3 0.85117     444.44
  4 0.85017     371.686
  avg 0.8502     412.9439
mmcls on timm test export optimize train/e2e
medium 0 0.78655 0.78655 0.77873
  1 0.78264 0.78264 0.77873
  2 0.77717 0.77717 0.77952
  3 0.7889 0.78968 0.7889
  4 0.77717 0.77873 0.77561
  avg 0.782486 0.782955 0.780297
large 0 0.853 0.8535 0.85183
  1 0.8475 0.848 0.8445
  2 0.84417 0.84433 0.84417
  3 0.84 0.84133 0.8425
  4 0.81917 0.819 0.81933
  avg 0.840767 0.841233 0.840467
npz on timm test export optimize train/e2e
medium 0 0.78577 0.78655 0.78812
  1 0.81079 0.80844 0.80532
  2 0.79437 0.79359 0.79124
  3 0.80766 0.80844 0.80532
  4 0.79828 0.79828 0.80453
  avg 0.799375 0.799062 0.798905
large 0 0.85683 0.85733 0.85617
  1 0.8595 0.85933 0.859
  2 0.86433 0.86517 0.86333
  3 0.86317 0.863 0.86383
  4 0.83667 0.83667 0.836
  avg 0.8561 0.8563 0.855667

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

Copy link
Contributor

@sovrasov sovrasov 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 the update! Would model weights for deit_tiny recipe be compatible with 1.6 and 2.1 after these changes?

@wonjuleee
Copy link
Contributor Author

Thanks for the update! Would model weights for deit_tiny recipe be compatible with 1.6 and 2.1 after these changes?

Hi @sovrasov, thank you for the question. No, the model layers are renamed following timm implementation. So, we need to have a special converter for reusing 1.6 model weights like

  new_state_dict = OrderedDict()
  for key, value in state_dict.items():
      new_key = key
      new_key = re.sub("backbone.ln1", "backbone.norm", new_key)
      new_key = re.sub("ffn.layers.0.0", "mlp.fc1", new_key)
      new_key = re.sub("ffn.layers.1", "mlp.fc2", new_key)
      new_key = re.sub("ln", "norm", new_key)
      new_key = re.sub("projection", "proj", new_key)
      new_key = re.sub("layers", "blocks", new_key)
      new_state_dict[new_key] = value

@sovrasov
Copy link
Contributor

sovrasov commented Jul 9, 2024

Thanks for the update! Would model weights for deit_tiny recipe be compatible with 1.6 and 2.1 after these changes?

Hi @sovrasov, thank you for the question. No, the model layers are renamed following timm implementation. So, we need to have a special converter for reusing 1.6 model weights like

  new_state_dict = OrderedDict()
  for key, value in state_dict.items():
      new_key = key
      new_key = re.sub("backbone.ln1", "backbone.norm", new_key)
      new_key = re.sub("ffn.layers.0.0", "mlp.fc1", new_key)
      new_key = re.sub("ffn.layers.1", "mlp.fc2", new_key)
      new_key = re.sub("ln", "norm", new_key)
      new_key = re.sub("projection", "proj", new_key)
      new_key = re.sub("layers", "blocks", new_key)
      new_state_dict[new_key] = value

Does this PR include the mentioned weights loading method? We might need to think about backward compatibility taking the future Geti integration into account.

@wonjuleee
Copy link
Contributor Author

Thanks for the update! Would model weights for deit_tiny recipe be compatible with 1.6 and 2.1 after these changes?

Hi @sovrasov, thank you for the question. No, the model layers are renamed following timm implementation. So, we need to have a special converter for reusing 1.6 model weights like

  new_state_dict = OrderedDict()
  for key, value in state_dict.items():
      new_key = key
      new_key = re.sub("backbone.ln1", "backbone.norm", new_key)
      new_key = re.sub("ffn.layers.0.0", "mlp.fc1", new_key)
      new_key = re.sub("ffn.layers.1", "mlp.fc2", new_key)
      new_key = re.sub("ln", "norm", new_key)
      new_key = re.sub("projection", "proj", new_key)
      new_key = re.sub("layers", "blocks", new_key)
      new_state_dict[new_key] = value

Does this PR include the mentioned weights loading method? We might need to think about backward compatibility taking the future Geti integration into account.

This PR updates the pretrained weight provided by TIMM. So I instead added the logic for loading weights from pretrained npz file.
We decided to install both OTX 1.6 and 2.x in Geti without considering backward compatibility between them.

@github-actions github-actions bot added the TEST Any changes in tests label Jul 10, 2024
Copy link
Contributor

@harimkang harimkang left a comment

Choose a reason for hiding this comment

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

If this doesn't guarantee DeIT compatibility with OTX 1.6, that's fine. But if so, it would be nice if the load_from_otx_v1_ckpt function in vit.py would spit out a warning or error message.
If the compatibility issue is an easy one to solve, it would be nice to modify the function to also work with v1 weight.

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 work. It seems that previous timm version has conflict w/ current change. Could you specify timm version at pyproject.toml?

Copy link

codecov bot commented Jul 11, 2024

Codecov Report

Attention: Patch coverage is 75.72816% with 50 lines in your changes missing coverage. Please review.

Project coverage is 80.17%. Comparing base (d4a4601) to head (95f2bcc).
Report is 2 commits behind head on develop.

Files Patch % Lines
...lgo/classification/backbones/vision_transformer.py 72.18% 47 Missing ⚠️
src/otx/algo/classification/vit.py 91.89% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3719      +/-   ##
===========================================
- Coverage    80.51%   80.17%   -0.35%     
===========================================
  Files          251      252       +1     
  Lines        25488    25502      +14     
===========================================
- Hits         20521    20445      -76     
- Misses        4967     5057      +90     
Flag Coverage Δ
py310 80.14% <75.72%> (-0.35%) ⬇️
py311 79.90% <75.72%> (-0.61%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wonjuleee
Copy link
Contributor Author

If this doesn't guarantee DeIT compatibility with OTX 1.6, that's fine. But if so, it would be nice if the load_from_otx_v1_ckpt function in vit.py would spit out a warning or error message. If the compatibility issue is an easy one to solve, it would be nice to modify the function to also work with v1 weight.

added at 0704da1.

@github-actions github-actions bot added DEPENDENCY Any changes in any dependencies (new dep or its version) should be produced via Change Request on PM BUILD labels Jul 11, 2024
@wonjuleee
Copy link
Contributor Author

Thanks for your work. It seems that previous timm version has conflict w/ current change. Could you specify timm version at pyproject.toml?

fixed at 9acdfec.

pyproject.toml Outdated Show resolved Hide resolved
src/otx/algo/classification/vit.py Outdated Show resolved Hide resolved
src/otx/algo/classification/vit.py Outdated Show resolved Hide resolved
src/otx/algo/classification/vit.py Outdated Show resolved Hide resolved
src/otx/algo/classification/vit.py Show resolved Hide resolved
src/otx/recipe/classification/h_label_cls/deit_tiny.yaml Outdated Show resolved Hide resolved
@wonjuleee wonjuleee merged commit f6a17f3 into openvinotoolkit:develop Jul 12, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUILD DEPENDENCY Any changes in any dependencies (new dep or its version) should be produced via Change Request on PM TEST Any changes in tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants