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

Support configurable input size #3788

Merged
merged 47 commits into from
Aug 14, 2024

Conversation

eunwoosh
Copy link
Contributor

@eunwoosh eunwoosh commented Aug 5, 2024

Summary

This PR includes

  • adds configurable input size and adaptive input size to OTX.
  • refactor input size in model code.

Explanation

adds configurable input size feature to OTX

User can configure desired input size after this PR. input_size value in 'data' part of recipe is automatically passed to model unless model is already initialized before it's passed to the engine in API use case.
Also, user can use adaptive input size which finds appropriate input size based on dataset statistics. Also, user can force to use downscale only which doesn't change input size if bigger input size is considered as proper input size.
All interfaces for input size are added to OTXDataModule.
So CLI user can uses it by adding --data.input_size 1000 for configuring input size and --data.adaptive_input_size "auto" for adaptive input size.
API user can use it by setting arguments of OTXDataModule. If model is initialized in engine, input_size will be automatically set but if not, then user should set input_size argument of the model by himself or herself.

refactor input size in model code

Previously, how input_size is manged in model side is different depending on each model.
Now, it's unified. All models get input_size argument and keep it as attribute except zvp model which uses fixed input_size.
Model also initializes own module differently based on input_size if necessary.

How to test

  • otx train --config ... --data_root ... --data.input_size 1024
  • otx train --config ... --data_root ... --data.adaptive_input_size "auto | downscale"

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 OTX 2.0 labels Aug 5, 2024
@eunwoosh eunwoosh force-pushed the configurable_input_size branch from 8113046 to a40db40 Compare August 5, 2024 08:09
Copy link
Contributor

@goodsong81 goodsong81 left a comment

Choose a reason for hiding this comment

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

Nice work. A few points to mention are as follows:

  • hasatrr() based branch logics might be quickly rusted and tangled. Hope it would be based on formal interfaces in the future.
  • input_size term might be better to be more consistent in terms of usage

src/otx/algo/classification/huggingface_model.py Outdated Show resolved Hide resolved
src/otx/algo/classification/mobilenet_v3.py Outdated Show resolved Hide resolved
src/otx/algo/detection/rtdetr.py Outdated Show resolved Hide resolved
src/otx/algo/detection/yolox.py Outdated Show resolved Hide resolved
src/otx/cli/cli.py Outdated Show resolved Hide resolved
src/otx/core/data/module.py Outdated Show resolved Hide resolved
src/otx/core/data/utils/utils.py Outdated Show resolved Hide resolved
src/otx/core/model/action_classification.py Outdated Show resolved Hide resolved
src/otx/engine/utils/auto_configurator.py Outdated Show resolved Hide resolved
@github-actions github-actions bot added the DOC Improvements or additions to documentation label Aug 9, 2024
@eunwoosh eunwoosh marked this pull request as ready for review August 9, 2024 11:02
@eunwoosh eunwoosh requested a review from samet-akcay as a code owner August 9, 2024 11:02
harimkang
harimkang previously approved these changes Aug 13, 2024
wonjuleee
wonjuleee previously approved these changes Aug 13, 2024
sungchul2
sungchul2 previously approved these changes Aug 13, 2024
@eunwoosh eunwoosh dismissed stale reviews from sungchul2, wonjuleee, and harimkang via 900faaf August 13, 2024 09:59
harimkang
harimkang previously approved these changes Aug 13, 2024
sovrasov
sovrasov previously approved these changes Aug 13, 2024
@eunwoosh eunwoosh dismissed stale reviews from sovrasov and harimkang via 8e6f8f8 August 13, 2024 14:43
Copy link
Contributor

@sooahleex sooahleex left a comment

Choose a reason for hiding this comment

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

Looks good to me for h-label head!

@eunwoosh eunwoosh enabled auto-merge August 14, 2024 01:35
Copy link
Contributor

@goodsong81 goodsong81 left a comment

Choose a reason for hiding this comment

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

LGTM w/ one last comment: we may want to touch the mem cache max image size to make this feature more effective in training
(Maybe in another PR? :p)

@eunwoosh
Copy link
Contributor Author

LGTM w/ one last comment: we may want to touch the mem cache max image size to make this feature more effective in training (Maybe in another PR? :p)

yes, we can consider including that feature in the next release.

@harimkang harimkang added this to the 2.2.0 milestone Aug 14, 2024
@goodsong81
Copy link
Contributor

LGTM w/ one last comment: we may want to touch the mem cache max image size to make this feature more effective in training (Maybe in another PR? :p)

yes, we can consider including that feature in the next release.

FYI, the configured input size could be set to the max memcache image size.
image
image

@eunwoosh
Copy link
Contributor Author

eunwoosh commented Aug 14, 2024

@goodsong81 I understand your intention. If it doesn't need huge code change, maybe it can be included in this release. Thanks for comment :)

@eunwoosh eunwoosh added this pull request to the merge queue Aug 14, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 14, 2024
@eunwoosh eunwoosh added this pull request to the merge queue Aug 14, 2024
Merged via the queue into openvinotoolkit:develop with commit 0b5ed3b Aug 14, 2024
18 checks passed
@eunwoosh eunwoosh deleted the configurable_input_size branch August 14, 2024 06:24
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.

9 participants