Enhance XAI (CLI, ExplainParameters, test) #1941

negvet · 2023-03-24T14:25:43Z

Summary

Update XAI CLI. Add --process-saliency-maps and --explain-all-classes.
Add ExplainParameters entity used for explain task.
Recover XAI test. Make them runable at CI.
Minor refactoring (e.g. rename DetSaliencyMap to DetClassProbabilityMap, which is more specific and precise).

Sorry for putting it all in the same PR.

How to test

Corresponding tests added to integration/cli +e2e

Checklist

I submit my changes into the develop branch
I have added description of my changes into CHANGELOG
I have updated the documentation accordingly
I have added tests to cover my changes
I have linked related issues

License

I submit my code changes under the same MIT 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: MIT

JihwanEom · 2023-03-24T16:27:25Z

tests/e2e/test_api_xai_sanity.py

        "SSD": (13, 13),
        "YOLOX": (13, 13),
    }

    @e2e_pytest_api
    def test_inference_xai(self):
        with tempfile.TemporaryDirectory() as temp_dir:
-            hyper_parameters, model_template = self.setup_configurable_parameters(DEFAULT_DET_TEMPLATE_DIR, num_iters=2)
+            hyper_parameters, model_template = self.setup_configurable_parameters(
+                DEFAULT_DET_TEMPLATE_DIR, num_iters=100


Do we need 100 liters for test code? I think it's quiet long even with e2e..

Agree with @JihwanEom 's opinion. could we reduce this parameter?

Sorry for not paying required attention to it. I reduced to 15. I tested, this is the minimal num of iters to get somewhat trained model. Is it ok for e2e?

JihwanEom · 2023-03-24T16:29:44Z

tests/integration/cli/detection/test_detection.py

@@ -35,6 +39,12 @@
    "train_params": ["params", "--learning_parameters.num_iters", "1", "--learning_parameters.batch_size", "4"],
 }

+num_iters_per_model = {


Should we have below long interactions with integration test?

For the same point, we need to reduce the elapsed time for integration tests.

IMO, we just need to check functionality in the integration tests executed at every PR.
So, how about reducing num_iters?

Actually, there is a functionality in XAI, which suppose to generate saliency maps only for predictions. I need a trained model to test it. So if the model is not trained - no confident predictions (no threshold is passed).

As a solution, I moved this check into the e2e, keeping 1 iter for tests/integration/cli/detection/test_detection.py Does it work for you?

Moving it into e2e looks good to me

tests/test_suite/run_test_command.py

otx/cli/tools/explain.py

tests/test_suite/run_test_command.py

sungmanc

Just question: is it difficult to use InferenceParameters as like before?

Adding new entity to otx.api effects to the further Geti interface.
So, I prefer using pre-existed one

otx/api/entities/inference_parameters.py

sungmanc · 2023-03-27T05:09:01Z

tests/e2e/test_api_xai_sanity.py

        "SSD": (13, 13),
        "YOLOX": (13, 13),
    }

    @e2e_pytest_api
    def test_inference_xai(self):
        with tempfile.TemporaryDirectory() as temp_dir:
-            hyper_parameters, model_template = self.setup_configurable_parameters(DEFAULT_DET_TEMPLATE_DIR, num_iters=2)
+            hyper_parameters, model_template = self.setup_configurable_parameters(
+                DEFAULT_DET_TEMPLATE_DIR, num_iters=100


Agree with @JihwanEom 's opinion. could we reduce this parameter?

tests/e2e/test_api_xai_sanity.py

sungmanc · 2023-03-27T05:14:33Z

tests/integration/cli/detection/test_detection.py

@@ -35,6 +39,12 @@
    "train_params": ["params", "--learning_parameters.num_iters", "1", "--learning_parameters.batch_size", "4"],
 }

+num_iters_per_model = {


For the same point, we need to reduce the elapsed time for integration tests.

IMO, we just need to check functionality in the integration tests executed at every PR.
So, how about reducing num_iters?

eunwoosh

Thanks for your work! Mostly LGTM. But I want to ask you to check current test iteration can become lower as Jihwan said.

negvet · 2023-03-27T16:43:37Z

Just question: is it difficult to use InferenceParameters as like before?
Adding new entity to otx.api effects to the further Geti interface. So, I prefer using pre-existed one

InferenceParameters are still used, but for task.infer().
ExplainParameters are used only for task.explain(), new API. I believe new interface allow us to introduce new objects of our choice to parametrize it (Geti is still not using explain API). Do you agree?

As you see, currently task.infer API still loaded with explain functionality - so for it InferenceParameters include both inference and explain parameters (I do not use new ExplainParameters for task.infer).

On the other side ExplainParameters will be used specifically for XAI purposes by task.explain. When task.explain will be used by Geti (1.4-1.5?), then explain load will be removed from task.infer (together with explain parameters in InferenceParameters) to make it faster.

Plus, there are many things that potentially can be parametrized in XAI, therefore I want to keep it separated from InferenceParameters.

sungmanc · 2023-03-28T00:48:45Z

Just question: is it difficult to use InferenceParameters as like before?
Adding new entity to otx.api effects to the further Geti interface. So, I prefer using pre-existed one

InferenceParameters are still used, but for task.infer(). ExplainParameters are used only for task.explain(), new API. I believe new interface allow us to introduce new objects of our choice to parametrize it (Geti is still not using explain API). Do you agree?

As you see, currently task.infer API still loaded with explain functionality - so for it InferenceParameters include both inference and explain parameters (I do not use new ExplainParameters for task.infer).

On the other side ExplainParameters will be used specifically for XAI purposes by task.explain. When task.explain will be used by Geti (1.4-1.5?), then explain load will be removed from task.infer (together with explain parameters in InferenceParameters) to make it faster.

Plus, there is many things that potentially can be parametrized in XAI, therefore I want to keep it separate from InferenceParameters.

Thanks for kind explanation.

tests/unit/algorithms/classification/test_xai_classification_validity.py

GalyaZalesskaya

Thank you for your work! Added 5 cents from my side.

GalyaZalesskaya · 2023-03-28T16:18:03Z

otx/cli/tools/explain.py

-        # "-w",
+        "--process-saliency-maps",
+        action="store_true",
+        help="Processing of saliency map includes (1) resize to input image resolution and (2) apply a colormap."


Suggested change

help="Processing of saliency map includes (1) resize to input image resolution and (2) apply a colormap."

help="Processing of saliency map includes (1) resizing to input image resolution and (2) applying a colormap."

GalyaZalesskaya · 2023-03-28T16:30:16Z

otx/cli/tools/explain.py

@@ -65,18 +68,61 @@ def get_args():
        "For Openvino task, default method will be selected.",
    )
    parser.add_argument(
-        # "-w",
+        "--process-saliency-maps",


Can you please add these parameters to the documentation (CLI commands: Explain) to keep it up to date?

Also can you consider updating Changelog with you CLI update as well?

wonjuleee

LGTM, but please resolve conflicts and update CHANGELOG as Galina mentioned. I believe that the document update could be done by the another PR.

negvet · 2023-03-30T11:21:08Z

TODO: rebase and update changelog

tests/e2e/test_api_xai_sanity.py

negvet · 2023-04-05T07:33:52Z

@JihwanEom @wonjuleee @sungmanc @GalyaZalesskaya Please review. I already rebased this PR three times due to conflicts. I would like it to avoid further rebasing if possible. Many thanks!

JihwanEom · 2023-04-05T16:25:42Z

@negvet Could you create another PR to resolve pre-commit issue? https://github.com/openvinotoolkit/training_extensions/actions/runs/4619805865/jobs/8169044264

negvet requested a review from a team as a code owner March 24, 2023 14:25

github-actions bot added 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 labels Mar 24, 2023

JihwanEom reviewed Mar 24, 2023

View reviewed changes

sungmanc reviewed Mar 27, 2023

View reviewed changes

sungmanc added this to the 1.2.0 milestone Mar 27, 2023

eunwoosh reviewed Mar 27, 2023

View reviewed changes

negvet force-pushed the et/xai_enhance branch from 2f2ab4d to 32d851b Compare March 27, 2023 16:23

negvet requested review from wonjuleee and GalyaZalesskaya March 27, 2023 16:24

negvet requested review from JihwanEom, sungmanc and eunwoosh March 27, 2023 17:22

eunwoosh previously approved these changes Mar 28, 2023

View reviewed changes

JihwanEom reviewed Mar 28, 2023

View reviewed changes

tests/unit/algorithms/classification/test_xai_classification_validity.py Show resolved Hide resolved

GalyaZalesskaya reviewed Mar 28, 2023

View reviewed changes

wonjuleee approved these changes Mar 28, 2023

View reviewed changes

wonjuleee previously approved these changes Mar 28, 2023

View reviewed changes

negvet dismissed stale reviews from wonjuleee and eunwoosh via be685cb March 30, 2023 11:15

negvet force-pushed the et/xai_enhance branch from 32d851b to be685cb Compare March 30, 2023 11:15

github-actions bot added the DOC Improvements or additions to documentation label Mar 30, 2023

negvet added 3 commits April 4, 2023 21:28

xai_enhance

36dfd36

Update IExplainTask signature

b1757f2

pylint fix

2acae2f

negvet added 4 commits April 4, 2023 21:28

logger

30bc432

reduce num_iters, rename DetClassProbMapHook

73b8799

reply to comments

de9cf87

rebase + docs

154e179

negvet force-pushed the et/xai_enhance branch from be685cb to 75c1b6e Compare April 4, 2023 16:19

rebase

75c1b6e

negvet requested review from GalyaZalesskaya, wonjuleee, JihwanEom and eunwoosh April 4, 2023 18:17

JihwanEom reviewed Apr 5, 2023

View reviewed changes

tests/e2e/test_api_xai_sanity.py Show resolved Hide resolved

negvet requested a review from JihwanEom April 5, 2023 10:45

wonjuleee approved these changes Apr 5, 2023

View reviewed changes

JihwanEom approved these changes Apr 5, 2023

View reviewed changes

sovrasov approved these changes Apr 5, 2023

View reviewed changes

sovrasov merged commit 40094c2 into openvinotoolkit:develop Apr 5, 2023

negvet deleted the et/xai_enhance branch July 18, 2023 14:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance XAI (CLI, ExplainParameters, test) #1941

Enhance XAI (CLI, ExplainParameters, test) #1941

negvet commented Mar 24, 2023 •

edited

Loading

JihwanEom Mar 24, 2023

sungmanc Mar 27, 2023

negvet Mar 27, 2023

JihwanEom Mar 24, 2023

sungmanc Mar 27, 2023

negvet Mar 27, 2023

sungmanc Mar 28, 2023

sungmanc left a comment

sungmanc Mar 27, 2023

sungmanc Mar 27, 2023

eunwoosh left a comment

negvet commented Mar 27, 2023 •

edited

Loading

sungmanc commented Mar 28, 2023

GalyaZalesskaya left a comment

GalyaZalesskaya Mar 28, 2023

GalyaZalesskaya Mar 28, 2023

wonjuleee left a comment

negvet commented Mar 30, 2023

negvet commented Apr 5, 2023

JihwanEom commented Apr 5, 2023

	help="Processing of saliency map includes (1) resize to input image resolution and (2) apply a colormap."
	help="Processing of saliency map includes (1) resizing to input image resolution and (2) applying a colormap."

Enhance XAI (CLI, ExplainParameters, test) #1941

Enhance XAI (CLI, ExplainParameters, test) #1941

Conversation

negvet commented Mar 24, 2023 • edited Loading

Summary

How to test

Checklist

License

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sungmanc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eunwoosh left a comment

Choose a reason for hiding this comment

negvet commented Mar 27, 2023 • edited Loading

sungmanc commented Mar 28, 2023

GalyaZalesskaya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wonjuleee left a comment

Choose a reason for hiding this comment

negvet commented Mar 30, 2023

negvet commented Apr 5, 2023

JihwanEom commented Apr 5, 2023

negvet commented Mar 24, 2023 •

edited

Loading

negvet commented Mar 27, 2023 •

edited

Loading