From a900e4a2d60278db211f9439a4f910e0aa53073b Mon Sep 17 00:00:00 2001
From: Kaleidophon <dennis.ulmer@gmx.de>
Date: Wed, 27 Oct 2021 17:59:24 +0200
Subject: [PATCH] :bug: Fix critical bugs related to multi_aso() and symmetry
 discovered via issue #7 (see below)

* Fix bug where symmetry property wouldn't be used to correctly to fill eps_min matrix
* Fix bug where indices would be misaligned, filling the wrong matrix entries with scores
* Add Mike's example from issue #7 as explicit test cases
---
 deepsig/__init__.py       |  2 +-
 deepsig/aso.py            |  4 +--
 deepsig/tests/test_aso.py | 51 +++++++++++++++++++++++++++++++++++++--
 setup.py                  |  2 +-
 4 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/deepsig/__init__.py b/deepsig/__init__.py
index 949d232..8f39f22 100644
--- a/deepsig/__init__.py
+++ b/deepsig/__init__.py
@@ -4,5 +4,5 @@
 from deepsig.correction import bonferroni_correction
 from deepsig.permutation import permutation_test
 
-__version__ = "1.1.2"
+__version__ = "1.1.3"
 __author__ = "Dennis Ulmer"
diff --git a/deepsig/aso.py b/deepsig/aso.py
index 7e85251..e5d062d 100644
--- a/deepsig/aso.py
+++ b/deepsig/aso.py
@@ -256,7 +256,7 @@ def multi_aso(
         )
 
     for i, key_i in enumerate(indices):
-        for j, key_j in enumerate(indices[(i + 1) :]):
+        for j, key_j in enumerate(indices[(i + 1) :], start=i + 1):
             scores_a, scores_b = scores[key_i], scores[key_j]
 
             eps_min[i, j] = aso(
@@ -274,7 +274,7 @@ def multi_aso(
 
             # Use ASO(A, B, alpha) = 1 - ASO(B, A, alpha)
             if use_symmetry:
-                eps_min[j, i] = eps_min[i, j]
+                eps_min[j, i] = 1 - eps_min[i, j]
 
             # Compute ASO(B, A, alpha) separately
             else:
diff --git a/deepsig/tests/test_aso.py b/deepsig/tests/test_aso.py
index 803422c..ba401d8 100644
--- a/deepsig/tests/test_aso.py
+++ b/deepsig/tests/test_aso.py
@@ -164,6 +164,13 @@ def setUp(self) -> None:
         self.scores_dict = {
             "model{}".format(i): scores for i, scores in enumerate(self.scores)
         }
+        # Test case based on https://github.com/Kaleidophon/deep-significance/issues/7
+        self.mikes_scores_dict = {
+            "x": np.array([59.13, 58.03, 59.18, 58.78, 58.5]),
+            "y": np.array([58.13, 59.19, 59.94, 60.08, 59.85]),
+            "z": np.array([58.77, 58.86, 59.58, 59.59, 59.64]),
+            "w": np.array([58.16, 58.49, 59.87, 58.94, 58.96]),
+        }
         self.scores_numpy = np.array(self.scores)
         self.scores_torch = torch.from_numpy(self.scores_numpy)
         self.scores_tensorflow = tf.convert_to_tensor(self.scores_numpy)
@@ -204,8 +211,48 @@ def test_symmetry(self):
         )
         symmetric_scores = multi_aso(self.scores_numpy, seed=seed, **self.aso_kwargs)
 
-        self.assertTrue(np.all(symmetric_scores == symmetric_scores.T))
-        self.assertTrue(np.any(asymmetric_scores != asymmetric_scores.T))
+        self.assertTrue(
+            np.all(
+                np.tril(symmetric_scores, -1) == np.tril((1 - symmetric_scores).T, -1)
+            )
+        )
+        self.assertTrue(
+            np.any(
+                np.tril(asymmetric_scores, -1) == np.tril((1 - asymmetric_scores).T, -1)
+            )
+        )
+        self.assertTrue(
+            np.all(np.diag(symmetric_scores) == 1)
+        )  # Check all diagonals to be one
+        self.assertTrue(
+            np.all(np.diag(asymmetric_scores) == 1)
+        )  # Check all diagonals to be one
+
+        # Cover Mike's test case: https://github.com/Kaleidophon/deep-significance/issues/7
+        mikes_asymmetric_scores = multi_aso(
+            self.mikes_scores_dict, seed=seed, use_symmetry=False, **self.aso_kwargs
+        )
+        mikes_symmetric_scores = multi_aso(
+            self.mikes_scores_dict, seed=seed, **self.aso_kwargs
+        )
+        self.assertTrue(
+            np.all(
+                np.tril(mikes_symmetric_scores, -1)
+                == np.tril((1 - mikes_symmetric_scores).T, -1)
+            )
+        )
+        self.assertTrue(
+            np.any(
+                np.tril(mikes_asymmetric_scores, -1)
+                == np.tril((1 - mikes_asymmetric_scores).T, -1)
+            )
+        )
+        self.assertTrue(
+            np.all(np.diag(mikes_symmetric_scores) == 1)
+        )  # Check all diagonals to be one
+        self.assertTrue(
+            np.all(np.diag(mikes_asymmetric_scores) == 1)
+        )  # Check all diagonals to be one
 
     def test_result_df(self):
         """
diff --git a/setup.py b/setup.py
index 1935c48..6ab96a7 100644
--- a/setup.py
+++ b/setup.py
@@ -8,7 +8,7 @@
 
 setup(
     name="deepsig",
-    version="1.1.2",
+    version="1.1.3",
     author="Dennis Ulmer",
     description="Easy Significance Testing for Deep Neural Networks.",
     long_description=long_description,