Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

[OTE / XAI] Handle two stage detector in the inferrer.py #104

Merged
merged 7 commits into from
Dec 13, 2022
Merged

[OTE / XAI] Handle two stage detector in the inferrer.py #104

merged 7 commits into from
Dec 13, 2022

Conversation

dongkwan-kim01
Copy link
Contributor

This is the hot-fix while running the Geti side OTE inference, fix handling the detection saliency map hook in the inferrer.py.
image

@dongkwan-kim01
[XAI] hot-fix of error in Detection XAI support (#98)
540ea47 
* add simple_test two-stage

* two-path for saliency map exporting

* two-stage detector exception for simple test

* remove assertion for class-wise saliency map

* fix assertion error in the tiling det
@dongkwan-kim01
handle two stage detectorin the inferrer.py
381c3b0
@dongkwan-kim01
handle two stage detectorin the inferrer.py
aefe7ea
@dongkwan-kim01
Merge branch 'dk/fix_det_saliency_map' of https://github.com/openvino…
374cbb7 
…toolkit/model_preparation_algorithm into dk/fix_det_saliency_map
@dongkwan-kim01
handle two stage detectorin the inferrer.py
bc4c4d3
goodsong81
goodsong81 suggested changes Dec 12, 2022
View reviewed changes
Copy link
Collaborator

@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.

Please redirect to releases/OTE_0.4.0 branch

@goodsong81
Copy link
Collaborator

Could you add a test to reproduce this bug? We couldn't find this in OTE CI.

@dongkwan-kim01
Copy link
Contributor Author

dongkwan-kim01 commented Dec 12, 2022 •
edited
Loading

@goodsong81 Yes, I've changed the branch to release.

Could you add a test to reproduce this bug? We couldn't find this in OTE CI.

Yes, seems strange why it didn't occur in the OTE test.. I'll also check and add it.

@dongkwan-kim01 dongkwan-kim01 changed the base branch from develop to releases/OTE_0.4.0 December 12, 2022 02:51
@dongkwan-kim01 dongkwan-kim01 force-pushed the dk/fix_det_saliency_map branch from 01b3f45 to 381c3b0 Compare December 12, 2022 02:59
@harimkang
Copy link
Contributor

Thank you for your hard work!
When will this be reproduced?

goodsong81
goodsong81 reviewed Dec 12, 2022
View reviewed changes
mpa/det/inferrer.py Outdated Show resolved Hide resolved
@dongkwan-kim01
Copy link
Contributor Author

@harimkang @goodsong81
Hi, I'm checking the test code but currently can't find the reason why two-stage detectors are not making errors in the previous PR and passed the test codes. I'll share you when I find the reason.

@dongkwan-kim01
Copy link
Contributor Author

@harimkang @goodsong81 The reason why the test code has passed is currently dump_saliency_map parameter is False for all tests in the OTE cli side, so actually all the saliency maps are None when we run the ote cli(train, eval, ...)
@negvet is writing for the test PR so I think it'll be resolved soon.

@harimkang
Copy link
Contributor

@harimkang @goodsong81 The reason why the test code has passed is currently dump_saliency_map parameter is False for all tests in the OTE cli side, so actually all the saliency maps are None when we run the ote cli(train, eval, ...) @negvet is writing for the test PR so I think it'll be resolved soon.

It would be nice if tests task.infer of one sample with dump_saliency_map=True is added to the ote cli test.

goodsong81
goodsong81 approved these changes Dec 12, 2022
View reviewed changes
Copy link
Collaborator

@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.

Changes look good to me but could you create OTE PR w/ some tests for this PR and run CI tests?

@dongkwan-kim01
Copy link
Contributor Author

@harimkang @goodsong81 Yes, I've made a PR to check from a single sample saliency map exporting step, though it could be improved in the test_xai improvement PR

@negvet
Copy link
Contributor

negvet commented Dec 12, 2022

It would be nice if tests task.infer of one sample with dump_saliency_map=True is added to the ote cli test.

dump_saliency_map is defined by InferenceParameters instance:
dump_saliency_map = not inference_parameters.is_evaluation if inference_parameters else True
We cannot control it from the CLI level AFAICS. Can we?

But we have some task.infer API tests https://github.com/openvinotoolkit/training_extensions/blob/46b831bcdb7db8e3dc08e819c37969de55c53b4d/external/model-preparation-algorithm/tests/api_tests/test_ote_detection_api.py#L221-L253 that actually run inference with dump_saliency_map = True (inference_parameters is None). The problem is that it is only for single stage detector DEFAULT_DET_TEMPLATE_DIR = osp.join("configs", "detection", "mobilenetv2_atss_cls_incr").

To test this PR we need task.infer API tests for two-stage detectors, which we do not have running for now, as I can see. Or am I missing it?

@harimkang
Copy link
Contributor

To test this PR we need task.infer API tests for two-stage detectors, which we do not have running for now, as I can see. Or am I missing it?

Yes, that's right. I think our test is missing the API test for Mask-RCNN (2 stage detector).

@goodsong81
Copy link
Collaborator

Let's handle missing tests in another PRs. I'm merging this PR.

@goodsong81 goodsong81 merged commit 5d4616e into releases/OTE_0.4.0 Dec 13, 2022
@goodsong81 goodsong81 deleted the dk/fix_det_saliency_map branch December 13, 2022 00:51
@dongkwan-kim01
Copy link
Contributor Author

I might be mistaken, but aren't we testing the two-stage detectors in the test_instance_segmentation.py?

@dongkwan-kim01 dongkwan-kim01 mentioned this pull request Dec 13, 2022
Merged
goodsong81 added a commit that referenced this pull request Dec 20, 2022
@goodsong81 @dongkwan-kim01 @supersoob
Merge back releases/OTE_0.4.0 to develop (#116)
15f73b6 
* [XAI] hot-fix of error in Detection XAI support (#99)

* add simple_test two-stage

* two-path for saliency map exporting

* two-stage detector exception for simple test

* remove assertion for class-wise saliency map

* fix assertion error in the tiling det

* [OTE / XAI] Handle two stage detector in the inferrer.py (#104)

* [XAI] hot-fix of error in Detection XAI support (#98)

* add simple_test two-stage

* two-path for saliency map exporting

* two-stage detector exception for simple test

* remove assertion for class-wise saliency map

* fix assertion error in the tiling det

* handle two stage detectorin the inferrer.py

* handle two stage detectorin the inferrer.py

* handle two stage detectorin the inferrer.py

* handle from detector instance checking

* accept tuple data

* [Hot-fix] Fix zero-division error in one cycle lr scheduler in multilabel classification

Co-authored-by: dongkwan-kim <[email protected]>
Co-authored-by: Soobee Lee <[email protected]>
goodsong81 added a commit that referenced this pull request Dec 28, 2022
@goodsong81 @kprokofi
@eugene123tw @sungmanc @JihwanEom @yunchu @sovrasov @negvet @dongkwan-kim01 @arrufat @supersoob
[MPA/OTX] Apply changes in develop to otx branch (#105)
1886385 
* fixed multilabel configs (#67)

* Tiling Module (#40)

* Feature/val batch and seed (#69)

* workaround bug (#70)

* Kp/devide runners (#71)

* move ema model to hook (#73)

* Appley release/OTE_0.3.0 changes (#77)

* Per-group softmax output (#74)

* Move drop_last in cls trainer.py (#79)

* Removed unnecessary mim version constraint for networkx package (#80)

* Revert "Removed unnecessary mim version constraint for networkx package (#80)" (#82)

* Don't apply activations on export in classification (#83)

* Delete unused code (#84)

* Replace current saliency map generation with Recipro-CAM for cls (#81)

* Class-wise saliency map generation for the detection task (#97)

* [XAI] hot-fix of error in Detection XAI support (#98)

* [XAI] hot-fix of error in Detection XAI support (#99)

* Replace only the first occurrence in the state_dict keys (#91)

* [OTE / XAI] Handle two stage detector in the inferrer.py (#104)

* [XAI] hot-fix of error in Detection XAI support (#98)

* [OTE / XAI][Develop] Handle two stage detector in the inferrer.py (#107)

* [XAI] hot-fix of error in Detection XAI support (#99)

* Merge OTE side XAI update to OTX (#109)

* Merge back releases/OTE_0.4.0 to develop (#116)

* [XAI] hot-fix of error in Detection XAI support (#99)

* [OTE / XAI] Handle two stage detector in the inferrer.py (#104)

* [XAI] hot-fix of error in Detection XAI support (#98)

* [Hot-fix] Fix zero-division error in one cycle lr scheduler in multilabel classification

* Fix extra activations when exporting nonlinear hierarchical head (#118)

* Fix get_train_data_cfg -> get_data_cfg

Signed-off-by: Songki Choi <[email protected]>
Co-authored-by: Prokofiev Kirill <[email protected]>
Co-authored-by: Eugene Liu <[email protected]>
Co-authored-by: Sungman Cho <[email protected]>
Co-authored-by: Jihwan Eom <[email protected]>
Co-authored-by: Yunchu Lee <[email protected]>
Co-authored-by: Vladislav Sovrasov <[email protected]>
Co-authored-by: Evgeny Tsykunov <[email protected]>
Co-authored-by: dongkwan-kim <[email protected]>
Co-authored-by: Adria Arrufat <[email protected]>
Co-authored-by: Soobee Lee <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@negvet negvet negvet approved these changes

@goodsong81 goodsong81 goodsong81 approved these changes

@harimkang harimkang Awaiting requested review from harimkang

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dongkwan-kim01 @goodsong81 @harimkang @negvet