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.

Sign up for GitHub

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

Jump to bottom

Do not skip full img tile classifier + Fix Sequencial Export Issue #2174

Merged
merged 20 commits into from
May 24, 2023
Merged

Do not skip full img tile classifier + Fix Sequencial Export Issue #2174

merged 20 commits into from
May 24, 2023

Conversation

eugene123tw
Copy link
Collaborator

@eugene123tw eugene123tw commented May 22, 2023 •
edited
Loading

Summary

  • Always predict full res image samples when the tile classifier is enabled as it may skip full-resolution images.
  • Fix sequential model export issue by resetting partition count for tile classifier

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

@eugene123tw eugene123tw marked this pull request as ready for review May 22, 2023 15:16
@eugene123tw eugene123tw requested a review from a team as a code owner May 22, 2023 15:16
@eugene123tw eugene123tw requested a review from goodsong81 May 22, 2023 15:17
@github-actions github-actions bot added the ALGO Any changes in OTX Algo Tasks implementation label May 22, 2023
@github-actions github-actions bot added the API Any changes in OTX API label May 22, 2023
jaegukhyun
jaegukhyun reviewed May 23, 2023
View reviewed changes
Copy link
Contributor

@jaegukhyun jaegukhyun left a comment

Choose a reason for hiding this comment

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

I left a comment

@eugene123tw eugene123tw requested a review from jaegukhyun May 23, 2023 08:02
jaegukhyun
jaegukhyun previously approved these changes May 23, 2023
View reviewed changes
@goodsong81 goodsong81 added HOTFIX FIX defect fix labels May 23, 2023
@eugene123tw eugene123tw changed the title [Bug] Do not skip full img tile classifier [Bug] Do not skip full img tile classifier + Fix Sequencial Export Issue May 23, 2023
@eugene123tw eugene123tw requested a review from sovrasov May 23, 2023 18:28
@eugene123tw eugene123tw requested a review from jaegukhyun May 23, 2023 19:50
jaegukhyun
jaegukhyun previously approved these changes May 24, 2023
View reviewed changes
@jaegukhyun
Copy link
Contributor

Let's merge this PR after merging tiling optimization PR

sungmanc
sungmanc previously approved these changes May 24, 2023
View reviewed changes
@jaegukhyun jaegukhyun force-pushed the eugene/CVS-111650-do-not-skip-full-img-tile-classifier branch from 71509f3 to 6063580 Compare May 24, 2023 06:30
jaegukhyun
jaegukhyun previously approved these changes May 24, 2023
View reviewed changes
sungmanc
sungmanc previously approved these changes May 24, 2023
View reviewed changes
@eugene123tw eugene123tw dismissed stale reviews from sungmanc and jaegukhyun via 6353c66 May 24, 2023 10:13
@eugene123tw
Copy link
Collaborator Author

image

jaegukhyun
jaegukhyun reviewed May 24, 2023
View reviewed changes
otx/algorithms/detection/adapters/mmdet/models/detectors/custom_maskrcnn_tile_optimized.py
keep = self.tile_classifier.simple_test(img) > 0.45
if isinstance(full_res_image, bool):
full_res_image = [full_res_image]
keep = full_res_image[0] | keep
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain when full_res_image is not bool?

Copy link
Collaborator Author

@eugene123tw eugene123tw May 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

mmdetection collects full_res_image as list. Only NNCF collate it as a single boolean

@eugene123tw eugene123tw requested a review from sungmanc May 24, 2023 13:19
@goodsong81 goodsong81 changed the title [Bug] Do not skip full img tile classifier + Fix Sequencial Export Issue Do not skip full img tile classifier + Fix Sequencial Export Issue May 24, 2023
@goodsong81 goodsong81 merged commit 8ddb18f into openvinotoolkit:releases/1.2.1 May 24, 2023
goodsong81 pushed a commit that referenced this pull request May 24, 2023
@eugene123tw
Do not skip full img tile classifier + Fix Sequencial Export Issue (#…
f1d1292 
…2174)

* add fixes

* quality checks

* fix mypy issue

* fix test

* fix bug

* fix sequencial export issue

* fix export bug

* add quality check

* fix nncf bug

* add fixes

* quality checks

* fix mypy issue

* fix test

* fix bug

* fix sequencial export issue

* fix export bug

* add quality check

* fix nncf bug

* fix nncf test
goodsong81 pushed a commit that referenced this pull request Jun 1, 2023
@sungmanc @jaegukhyun
@eugene123tw @sovrasov
Merge back 1.2.3 -> 1.3.1 (#2192)
2274cea 
* Optimize counting train & inference speed and memory consumption (#2172)

* Do not skip full img tile classifier + Fix Sequencial Export Issue (#2174)

* Add warning message to tiling parameter (#2193)

* Change the way to pad for instance-segmentation

* Hotfix: use 0 confidence threshold when computing best threshold based on F1

* Fix possible bw issue in exportable code (#2212)

* Fix ellipse load (#2214)

---------

Signed-off-by: Songki Choi <songki.choi@intel.com>
Co-authored-by: Sungman Cho <sungman.cho@intel.com>
Co-authored-by: jaegukhyun <jaeguk.hyun@intel.com>
Co-authored-by: Eugene Liu <eugene.liu@intel.com>
Co-authored-by: sungmanc <sungmanc@intel.com>
Co-authored-by: Vladislav Sovrasov <sovrasov.vlad@gmail.com>
goodsong81 pushed a commit that referenced this pull request Jun 5, 2023
@jaegukhyun @vinnamkim
@sungmanc @eunwoosh @sungchul2 @yunchu @JihwanEom @eugene123tw @sovrasov @djdameln
Merge back 1.3.1 to develop (#2218)
dbeb59a 
* Fix e2e (#2179)

* Fix iseg e2e (#2173)

* Fix det e2e (#2176)

* Update noisy label det docs for detection task (#2183)

* Fix iseg e2e (#2173)

* Fix det e2e (#2176)

* Add supcon seg for regression (#2177)

* Fix iseg e2e (#2173)

* Fix det e2e (#2176)

* Update regression test config (#2186)

* Fix for anomaly regression test (#2188)

* Fix iseg regression (#2190)

* Set path to save pseudo masks into workspace (#2185)

* Temporary fix override_configs bug (#2187)

* Fix iseg e2e (#2173)

* Fix det e2e (#2176)

* Cleaned up skip/xfail TCs on the classification e2e tests (#2196)

* Added csv output to export dependencies (#2178)

* Fix typo and enlarge the threshold for KPI tests (#2198)

* bugfix: auto adapt batch size doesn't work with cls incr case (#2199)

* Fix for ssd (#2197)

* Turn off persistent worker if num_worker is zero in classification task (#2208)

* Update test cases of common_semantic_segmentation_dataset (#2210)

* Optimize counting train & inference speed and memory consumption (#2172)

* Do not skip full img tile classifier + Fix Sequencial Export Issue (#2174)

* Add warning message to tiling parameter (#2193)

* Change the way to pad for instance-segmentation

* Hotfix: use 0 confidence threshold when computing best threshold based on F1

* Fix possible bw issue in exportable code (#2212)

* Fix ellipse load (#2214)

* Fix GT vs prediction order mismatch due to different sorting result (#2216)

* Return raw anomaly maps as metadata (#2217)

* Fix merge error, revert to pymongo w/o version spec

---------

Signed-off-by: Songki Choi <songki.choi@intel.com>
Signed-off-by: Kim, Vinnam <vinnam.kim@intel.com>
Co-authored-by: Jaeguk Hyun <jaeguk.hyun@intel.com>
Co-authored-by: Vinnam Kim <vinnam.kim@intel.com>
Co-authored-by: Sungman Cho <sungman.cho@intel.com>
Co-authored-by: Eunwoo Shin <eunwoo.shin@intel.com>
Co-authored-by: Kim, Sungchul <sungchul.kim@intel.com>
Co-authored-by: Yunchu Lee <yunchu.lee@intel.com>
Co-authored-by: Jihwan Eom <jihwan.eom@intel.com>
Co-authored-by: Eugene Liu <eugene.liu@intel.com>
Co-authored-by: sungmanc <sungmanc@intel.com>
Co-authored-by: Vladislav Sovrasov <sovrasov.vlad@gmail.com>
Co-authored-by: Dick Ameln <dick.ameln@intel.com>
@eugene123tw eugene123tw deleted the eugene/CVS-111650-do-not-skip-full-img-tile-classifier branch May 7, 2024 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sovrasov sovrasov sovrasov left review comments

@jaegukhyun jaegukhyun jaegukhyun approved these changes

@goodsong81 goodsong81 goodsong81 approved these changes

@sungmanc sungmanc Awaiting requested review from sungmanc

Assignees
No one assigned
Labels
ALGO Any changes in OTX Algo Tasks implementation API Any changes in OTX API FIX defect fix HOTFIX
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@eugene123tw @jaegukhyun @goodsong81 @sovrasov @sungmanc