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

Refactor model export part 2: Add a dedicated forward function for model export #3317

Merged

Conversation

vinnamkim
Copy link
Contributor

@vinnamkim vinnamkim commented Apr 12, 2024

Summary

  • Ticket No. 138400
  • Prior to this PR, there were hidden conditions where OTXModelExporter internally traced the OTXModel.model.forward function (not OTXModel.forward). This function needed to have a function signature such as Callable[[torch.Tensor], torch.Tensor | dict[str, torch.Tensor]] to correctly trace the model. Additionally, the auxiliary functions _reset_model_forward() and _restore_model_forward() were required to export an explainable model.
  • However, this was not easily visible to a new model developer and was somewhat messy in design. Therefore, a dedicated forward function, OTXModel.forward_for_tracing(), has been explicitly added. Any derived model that wants to use OTXNativeModelExporter for its model export can implement this function.

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) 2024 Intel Corporation
# SPDX-License-Identifier: Apache-2.0

@github-actions github-actions bot added TEST Any changes in tests OTX 2.0 labels Apr 12, 2024
Copy link

codecov bot commented Apr 12, 2024

Codecov Report

Attention: Patch coverage is 78.88199% with 34 lines in your changes are missing coverage. Please review.

Project coverage is 76.19%. Comparing base (c411844) to head (4b07554).
Report is 1 commits behind head on develop.

❗ Current head 4b07554 differs from pull request most recent head 6b7efc0. Consider uploading reports for the commit 6b7efc0 to get more accurate results

Files Patch % Lines
src/otx/core/model/base.py 80.76% 10 Missing ⚠️
src/otx/core/model/anomaly.py 20.00% 8 Missing ⚠️
src/otx/engine/engine.py 84.21% 6 Missing ⚠️
src/otx/algo/classification/torchvision_model.py 40.00% 3 Missing ⚠️
src/otx/core/model/classification.py 50.00% 3 Missing ⚠️
src/otx/core/model/utils/mmpretrain.py 90.47% 2 Missing ⚠️
src/otx/core/model/action_classification.py 50.00% 1 Missing ⚠️
src/otx/core/model/segmentation.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3317      +/-   ##
===========================================
- Coverage    77.15%   76.19%   -0.97%     
===========================================
  Files          204      194      -10     
  Lines        17526    16688     -838     
===========================================
- Hits         13523    12715     -808     
+ Misses        4003     3973      -30     
Flag Coverage Δ
py310 ?
py311 76.19% <78.88%> (-0.97%) ⬇️

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.

@vinnamkim vinnamkim force-pushed the refactor-export-forward-func branch 2 times, most recently from f3d6070 to eb1a773 Compare April 15, 2024 06:34
Signed-off-by: Kim, Vinnam <[email protected]>
Signed-off-by: Kim, Vinnam <[email protected]>
Signed-off-by: Kim, Vinnam <[email protected]>
Signed-off-by: Kim, Vinnam <[email protected]>
jaegukhyun
jaegukhyun previously approved these changes Apr 16, 2024
@vinnamkim vinnamkim merged commit 2e6d225 into openvinotoolkit:develop Apr 18, 2024
13 checks passed
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