Unify metric API #294

aaarrti · 2023-08-12T20:49:05Z

Description

Unify metric API, by removing the distinction between BatchedMetric and Metric, this way we will have centralized uniform pre- and post-processing as well as clearly defined API.
Some metrics can easily be batched, but this will be done in separate PR.

Implemented changes

Moved batched pre-processing logic to Metric, remove evaluate_instance anstract method.
BatchedMetric is not just an alias to Metric
Make instance-wise metric conform to the API defined by Metric

Changelist

The only place with any real changes are base.py (mostly copied over from base_batched.py)
All other changes in metrics are just wrapping evaluate_instance in for-loop.

Minimum acceptance criteria

No breaking changes.

codecov-commenter · 2023-08-12T20:57:20Z

Codecov Report

Attention: 67 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (main@1de69e6). Click here to learn what that means.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #294   +/-   ##
=======================================
  Coverage        ?   91.03%           
=======================================
  Files           ?       62           
  Lines           ?     3402           
  Branches        ?        0           
=======================================
  Hits            ?     3097           
  Misses          ?      305           
  Partials        ?        0

Files	Coverage Δ
quantus/__init__.py	`100.00% <100.00%> (ø)`
quantus/helpers/model/models.py	`84.29% <ø> (ø)`
quantus/helpers/model/pytorch_model.py	`87.41% <100.00%> (ø)`
quantus/helpers/model/tf_model.py	`91.60% <100.00%> (ø)`
quantus/helpers/utils.py	`92.28% <100.00%> (ø)`
quantus/metrics/__init__.py	`100.00% <100.00%> (ø)`
quantus/helpers/model/model_interface.py	`77.50% <87.50%> (ø)`
quantus/metrics/axiomatic/completeness.py	`93.02% <94.11%> (ø)`
quantus/metrics/axiomatic/input_invariance.py	`95.34% <93.75%> (ø)`
quantus/metrics/axiomatic/non_sensitivity.py	`96.00% <93.75%> (ø)`
... and 34 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

quantus/helpers/constants.py

quantus/metrics/axiomatic/completeness.py

quantus/metrics/axiomatic/input_invariance.py

annahedstroem

Review part 1/N.

dkrako

Great, this takes a good direction!

I'll try to block sime time to review in detail next week

annahedstroem

Really great work Artem, like the simple and straightfoward solution about looping over all evaluate_instance in the evaluate_batch. Solves the problem neatly. Do however put an issue for vectorisation as you've mentioned in several todos.

Almost ready for merge :muscle!

quantus/metrics/base.py

quantus/metrics/base_batched.py

quantus/metrics/base_perturbed.py

quantus/metrics/base.py

quantus/metrics/randomisation/model_parameter_randomisation.py

quantus/metrics/robustness/avg_sensitivity.py

quantus/metrics/robustness/consistency.py

quantus/metrics/robustness/continuity.py

quantus/metrics/robustness/relative_representation_stability.py

# Conflicts: # tests/metrics/test_localisation_metrics.py

annahedstroem · 2023-10-26T14:20:32Z

Is this ready to merge @aaarrti

@dkrako would you have the chance to take a quick look?

quantus/metrics/robustness/continuity.py

annahedstroem · 2023-10-27T13:29:11Z

quantus/metrics/robustness/consistency.py

+        ----------
+        args:
+            Unused.
+        a_batch:


@aaarrti missing docs, please please double-check now everywhere that it is consistent.

annahedstroem · 2023-10-27T13:30:55Z

quantus/metrics/robustness/avg_sensitivity.py

+# Quantus project URL: <https://github.com/understandable-machine-intelligence-lab/Quantus>.
+
+
+if sys.version_info >= (3, 8):


what is this? this is new?

@final shows users, that this class must not be inherited from.
E.g.,

from typing import final @final class A: pass class B(A): pass

mypy example.py mypy.ini: [mypy]: Unrecognized option: show_none_errors = False example.py:9: error: Cannot inherit from final class "A" [misc] Found 1 error in 1 file (checked 1 source file)

The if sys.version_info >= (3, 8) can be removed, after we drop python3.7 support (which I hope will be soon, because it is not maintained since half a year https://devguide.python.org/versions/)

dkrako

I just saw that there are mutable default parameters in a lot of metric constructors.

using mutable defaults for parameters is dangerous and can lead to very hard to find bugs:
https://florimond.dev/en/posts/2018/08/python-mutable-defaults-are-the-source-of-all-evil/

quantus/metrics/axiomatic/completeness.py

aaarrti · 2023-11-02T09:57:21Z

I just saw that there are mutable default parameters in a lot of metric constructors.

using mutable defaults for parameters is dangerous and can lead to very hard to find bugs: https://florimond.dev/en/posts/2018/08/python-mutable-defaults-are-the-source-of-all-evil/

@dkrako @annahedstroem Hi, I've realised every metric has had mutable default all this time, e.g., https://github.com/understandable-machine-intelligence-lab/Quantus/blob/main/quantus/metrics/axiomatic/completeness.py#L71. I will replace those with None also and add linter check for mutable defaults.

aaarrti · 2023-11-03T20:02:17Z

Really great work Artem, like the simple and straightfoward solution about looping over all evaluate_instance in the evaluate_batch. Solves the problem neatly. Do however put an issue for vectorisation as you've mentioned in several todos.

Almost ready for merge :muscle!

done #299

WIP

e0e090c

aaarrti added 6 commits August 12, 2023 22:59

WIP

8576b4d

WIP

bafbd93

WIP

764e415

WIP

e8b9f71

WIP

ae8e079

WIP

c16ff51

aaarrti changed the title ~~WIP~~ Unify metric API Aug 13, 2023

aaarrti requested a review from annahedstroem August 13, 2023 14:40

aaarrti marked this pull request as ready for review August 13, 2023 14:40

WIP

61baaa0

annahedstroem reviewed Aug 29, 2023

View reviewed changes

quantus/helpers/constants.py Show resolved Hide resolved

annahedstroem reviewed Aug 29, 2023

View reviewed changes

quantus/metrics/axiomatic/completeness.py Outdated Show resolved Hide resolved

annahedstroem reviewed Aug 29, 2023

View reviewed changes

quantus/metrics/axiomatic/input_invariance.py Show resolved Hide resolved

annahedstroem reviewed Aug 29, 2023

View reviewed changes

aaarrti added 10 commits August 30, 2023 16:04

minify git diff

121f0ef

minify git diff

394d417

minify git diff

41b8051

Merge branch 'main' into unfiy-metric-api

4582f14

minify git diff

2dff480

minify git diff

e968896

minify git diff

7acfd85

minify git diff

d08dc0f

mypy fixes

6989748

add missing docstrings

4d1d25e

annahedstroem requested a review from dkrako October 4, 2023 13:08

dkrako reviewed Oct 4, 2023

View reviewed changes

annahedstroem self-requested a review October 4, 2023 15:21

annahedstroem requested changes Oct 4, 2023

View reviewed changes

aaarrti added 12 commits October 17, 2023 19:25

* mypy fixes

9809d63

* add xfail

e37de15

* add xfail

fdfe768

* add xfail

adf3482

* add xfail

f85310a

* add xfail

0433a49

*

e537708

* revert typing changes, update docs

f0f90d9

* cleanup

aea87ad

* typing fix

c23a0ee

Merge branch 'main' into unfiy-metric-api

31d4ef9

# Conflicts: # tests/metrics/test_localisation_metrics.py

* typing fix

36dcd75

aaarrti and others added 3 commits October 27, 2023 12:45

Merge branch 'main' into unfiy-metric-api

10a78bc

* cleanup

d0abbf8

* bump up the version

2124abf

annahedstroem reviewed Oct 31, 2023

View reviewed changes

dkrako requested changes Nov 1, 2023

View reviewed changes

quantus/metrics/axiomatic/completeness.py Outdated Show resolved Hide resolved

aaarrti added 4 commits November 3, 2023 15:39

code review fixes

88e1ad0

* code review comments

fbefe31

* code review comments

27e908b

* code review comments

8284075

aaarrti added 3 commits November 3, 2023 21:26

* remove TypedDict

669c8ff

*

0f535f0

mypy fixes

a268d10

annahedstroem merged commit 1ed8b32 into main Nov 15, 2023
7 checks passed

aaarrti deleted the unfiy-metric-api branch December 23, 2023 19:14

aaarrti mentioned this pull request Jul 24, 2024

Adding "true" batch implementation to metrics #350

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unify metric API #294

Unify metric API #294

aaarrti commented Aug 12, 2023 •

edited

Loading

codecov-commenter commented Aug 12, 2023 •

edited

Loading

annahedstroem left a comment

dkrako left a comment

annahedstroem left a comment

annahedstroem commented Oct 26, 2023

annahedstroem Oct 27, 2023

annahedstroem Oct 31, 2023

annahedstroem Oct 27, 2023

annahedstroem Oct 31, 2023

aaarrti Nov 3, 2023

dkrako left a comment

aaarrti commented Nov 2, 2023

aaarrti commented Nov 3, 2023

		# Quantus project URL: <https://github.com/understandable-machine-intelligence-lab/Quantus>.


		if sys.version_info >= (3, 8):

Unify metric API #294

Unify metric API #294

Conversation

aaarrti commented Aug 12, 2023 • edited Loading

Description

Implemented changes

Changelist

Minimum acceptance criteria

codecov-commenter commented Aug 12, 2023 • edited Loading

Codecov Report

annahedstroem left a comment

Choose a reason for hiding this comment

dkrako left a comment

Choose a reason for hiding this comment

annahedstroem left a comment

Choose a reason for hiding this comment

annahedstroem commented Oct 26, 2023

annahedstroem Oct 27, 2023

Choose a reason for hiding this comment

annahedstroem Oct 31, 2023

Choose a reason for hiding this comment

annahedstroem Oct 27, 2023

Choose a reason for hiding this comment

annahedstroem Oct 31, 2023

Choose a reason for hiding this comment

aaarrti Nov 3, 2023

Choose a reason for hiding this comment

dkrako left a comment

Choose a reason for hiding this comment

aaarrti commented Nov 2, 2023

aaarrti commented Nov 3, 2023

aaarrti commented Aug 12, 2023 •

edited

Loading

codecov-commenter commented Aug 12, 2023 •

edited

Loading