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

Pre-work for the design change: create own python class for every model #2775

Merged

Conversation

vinnamkim
Copy link
Contributor

Summary

  • This is the pre-work, so that we will actually introduce own python class for every model in the next PR.
  • Enrich OTXModel to have export(), register_explain_hook(), and label_info property.
  • label_info will be automatically constructed by the input integer argument, num_classes.

How to test

Checklist

  • I have added unit tests to cover my changes.​
  • I have added integration tests to cover my changes.​
  • I have added e2e tests for validation.
  • 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) 2023 Intel Corporation
# SPDX-License-Identifier: Apache-2.0

@github-actions github-actions bot added TEST Any changes in tests OTX 2.0 labels Jan 11, 2024
@vinnamkim vinnamkim force-pushed the create-own-python-class-for-every-model branch from b708ad1 to bc6e7fc Compare January 11, 2024 06:49
Signed-off-by: Kim, Vinnam <vinnam.kim@intel.com>
Signed-off-by: Kim, Vinnam <vinnam.kim@intel.com>
Signed-off-by: Kim, Vinnam <vinnam.kim@intel.com>
Copy link

@MarkByun MarkByun left a comment

Choose a reason for hiding this comment

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

It looks good.

src/otx/core/data/dataset/base.py Show resolved Hide resolved
src/otx/core/utils/config.py Show resolved Hide resolved
src/otx/core/model/entity/base.py Show resolved Hide resolved
Copy link
Contributor

@sungmanc sungmanc left a comment

Choose a reason for hiding this comment

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

Approved, BTW, could you please explain why do we need to insert the num_classes to the OTXModel at this PR ? I'm not opposite to inject this argument, however, just curiosity. It seems that inserting the num_classes have no much relation with own python class.

@vinnamkim
Copy link
Contributor Author

Approved, BTW, could you please explain why do we need to insert the num_classes to the OTXModel at this PR ? I'm not opposite to inject this argument, however, just curiosity. It seems that inserting the num_classes have no much relation with own python class.

We introduce LabelInfo as a common property of OTXModel. However, this property affects the shape of the model itself. If we decide it lazily, not at the construction, we should implement this _reset_prediction_layer() as well this time. In addition, lazily resetting the layer seems not good in terms of the clean flow and computation cost.

@vinnamkim vinnamkim merged commit 02c3938 into openvinotoolkit:v2 Jan 11, 2024
6 checks passed
@vinnamkim vinnamkim deleted the create-own-python-class-for-every-model branch January 11, 2024 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TEST Any changes in tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants