Merge pull request #2738 from cta-observatory/fix_disp_sign

Fix wrong proba column being used for disp sign

Author: maxnoe

docs/changes/2738.bugfix.rst

@@ -0,0 +1 @@
+Fix inverted sign of ``DispReconstructor`` prediction.

src/ctapipe/conftest.py

@@ -642,6 +642,7 @@ def disp_reconstructor_path(model_tmp_path, gamma_train_clf):
     from ctapipe.tools.train_disp_reconstructor import TrainDispReconstructor
 
     out_file = model_tmp_path / "disp_reconstructor.pkl"
+    cv_out_file = model_tmp_path / "cv_disp_reconstructor.h5"
     with FileLock(out_file.with_suffix(out_file.suffix + ".lock")):
         if out_file.is_file():
             return out_file
@@ -653,12 +654,13 @@ def disp_reconstructor_path(model_tmp_path, gamma_train_clf):
             argv=[
                 f"--input={gamma_train_clf}",
                 f"--output={out_file}",
+                f"--cv-output={cv_out_file}",
                 f"--config={config}",
                 "--log-level=INFO",
             ],
         )
         assert ret == 0
-        return out_file
+        return out_file, cv_out_file
 
 
 @pytest.fixture(scope="session")

src/ctapipe/reco/sklearn.py

@@ -703,7 +703,7 @@ def _predict(self, key, table):
             else:
                 prediction[valid] = valid_norms
 
-            sign_proba = self._models[key][1].predict_proba(X)[:, 0]
+            sign_proba = self._models[key][1].predict_proba(X)[:, 1]
             # proba is [0 and 1] where 0 => very certain -1, 1 => very certain 1
             # and 0.5 means random guessing either. So we transform to a score
             # where 0 means "guessing" and 1 means "very certain"

src/ctapipe/tools/tests/test_apply_models.py

@@ -110,6 +110,8 @@ def test_apply_all(
 ):
     from ctapipe.tools.apply_models import ApplyModels
 
+    disp_reconstructor_path, _ = disp_reconstructor_path
+
     input_path = get_dataset_path("gamma_diffuse_dl2_train_small.dl2.h5")
     output_path = tmp_path / "particle-and-energy-and-disp.dl2.h5"
 

src/ctapipe/tools/tests/test_process_ml.py

@@ -122,6 +122,8 @@ def test_process_apply_disp(
 ):
     from ctapipe.tools.process import ProcessorTool
 
+    disp_reconstructor_path, _ = disp_reconstructor_path
+
     output = tmp_path / "gamma_prod5.dl2_disp.h5"
 
     config_path = tmp_path / "config.json"

src/ctapipe/tools/tests/test_train.py

@@ -3,6 +3,7 @@
 
 from ctapipe.core import ToolConfigurationError, run_tool
 from ctapipe.exceptions import TooFewEvents
+from ctapipe.io import read_table
 from ctapipe.utils.datasets import resource_file
 
 
@@ -21,7 +22,15 @@ def test_train_particle_classifier(particle_classifier_path):
 def test_train_disp_reconstructor(disp_reconstructor_path):
     from ctapipe.reco import DispReconstructor
 
-    DispReconstructor.read(disp_reconstructor_path)
+    model_path, cv_path = disp_reconstructor_path
+
+    DispReconstructor.read(model_path)
+
+    cv_table = read_table(cv_path, "/cv_predictions/LST_LST_LSTCam")
+    disp = cv_table["disp_parameter"]
+    true_disp = cv_table["truth"]
+    accuracy = np.count_nonzero(np.sign(disp) == np.sign(true_disp)) / len(disp)
+    assert accuracy > 0.75
 
 
 def test_too_few_events(tmp_path, dl2_shower_geometry_file):