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

[FEATURE] Add FP16 IR export support #1683

Merged
merged 6 commits into from
Feb 24, 2023
Merged

[FEATURE] Add FP16 IR export support #1683

merged 6 commits into from
Feb 24, 2023

Conversation

sovrasov
Copy link
Contributor

@sovrasov sovrasov commented Feb 13, 2023

Flag --half-precision was added to the export tool.

@harimkang @sungmanc please have a look at CLI changes

@github-actions github-actions bot added ALGO Any changes in OTX Algo Tasks implementation API Any changes in OTX API CLI Any changes in OTE CLI labels Feb 13, 2023
@sovrasov sovrasov changed the title [FEARURE] Add FP16 IR export support [FEATURE] Add FP16 IR export support Feb 13, 2023
@harimkang
Copy link
Contributor

The CLI changes don't seem to be a problem :)

@sovrasov sovrasov marked this pull request as ready for review February 14, 2023 22:01
@sovrasov sovrasov requested a review from a team as a code owner February 14, 2023 22:01
@github-actions github-actions bot added the TEST Any changes in tests label Feb 14, 2023
sungmanc
sungmanc previously approved these changes Feb 15, 2023
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.

LGTM, no problem with export CLI

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, but I think we should not include this PR in this release as we need additional validation. Let's merge after code fix.
(No request change, but to prevent merge :))

@goodsong81 goodsong81 added this to the 1.1.0 milestone Feb 17, 2023
@sovrasov sovrasov force-pushed the vs/fp16_export branch 2 times, most recently from 6760e4a to 8e1af9d Compare February 21, 2023 19:29
@codecov-commenter
Copy link

Codecov Report

Base: 78.44% // Head: 78.41% // Decreases project coverage by -0.04% ⚠️

Coverage data is based on head (d463e0b) compared to base (29560b9).
Patch coverage: 88.46% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1683      +/-   ##
===========================================
- Coverage    78.44%   78.41%   -0.04%     
===========================================
  Files          469      469              
  Lines        32833    32844      +11     
===========================================
- Hits         25757    25753       -4     
- Misses        7076     7091      +15     
Impacted Files Coverage Δ
otx/algorithms/common/tasks/training_base.py 50.57% <33.33%> (-0.15%) ⬇️
otx/algorithms/anomaly/tasks/inference.py 82.51% <75.00%> (-0.27%) ⬇️
...hms/action/adapters/mmaction/utils/export_utils.py 93.28% <100.00%> (+0.05%) ⬆️
otx/algorithms/action/tasks/inference.py 88.47% <100.00%> (+0.08%) ⬆️
otx/algorithms/classification/tasks/inference.py 86.19% <100.00%> (ø)
otx/algorithms/detection/tasks/inference.py 72.07% <100.00%> (-1.89%) ⬇️
otx/algorithms/segmentation/tasks/inference.py 88.96% <100.00%> (ø)
.../api/usecases/tasks/interfaces/export_interface.py 100.00% <100.00%> (ø)
otx/cli/tools/export.py 93.18% <100.00%> (+0.32%) ⬆️
...algorithms/detection/adapters/mmdet/data/tiling.py 79.82% <0.00%> (-5.58%) ⬇️
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sovrasov
Copy link
Contributor Author

@goodsong81 @sungmanc I think we could consider merging this, since the release branch was created

Copy link
Contributor

@kprokofi kprokofi left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, but I thing we should create FP16 export test

@sovrasov
Copy link
Contributor Author

Overall looks good to me, but I thing we should create FP16 export test

I'll address this in a separate PR, when FP16 inference on CI will be available.

@sovrasov sovrasov merged commit 0c7eae7 into develop Feb 24, 2023
@sovrasov sovrasov deleted the vs/fp16_export branch February 24, 2023 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ALGO Any changes in OTX Algo Tasks implementation API Any changes in OTX API CLI Any changes in OTE CLI TEST Any changes in tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants