From bba83f55173a8f7c5cc99c2d78203d14b62b3297 Mon Sep 17 00:00:00 2001
From: ahmedk <karim.ahmed@xfel.eu>
Date: Fri, 15 Mar 2024 12:32:51 +0100
Subject: [PATCH] fix get_bias_voltage by passing channel value and add
 comments to discuss in MR

---
 .../AGIPD/AGIPD_Correct_and_Verify.ipynb      | 21 ++++++++++++-------
 .../Characterize_AGIPD_Gain_Darks_NBC.ipynb   | 12 +++++------
 src/cal_tools/agipdlib.py                     | 16 +++++++++-----
 3 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/notebooks/AGIPD/AGIPD_Correct_and_Verify.ipynb b/notebooks/AGIPD/AGIPD_Correct_and_Verify.ipynb
index cf85b3e52..b5a01841c 100644
--- a/notebooks/AGIPD/AGIPD_Correct_and_Verify.ipynb
+++ b/notebooks/AGIPD/AGIPD_Correct_and_Verify.ipynb
@@ -388,13 +388,12 @@
    "outputs": [],
    "source": [
     "first_mod_channel = sorted(modules)[0]\n",
-    "\n",
-    "instrument_src_mod = [\n",
+    "instrument_src_first_mod = [\n",
     "    s for s in list(dc.all_sources) if f\"{first_mod_channel}CH\" in s][0]\n",
     "\n",
     "agipd_cond = AgipdCtrl(\n",
     "    run_dc=dc,\n",
-    "    image_src=instrument_src_mod,\n",
+    "    image_src=instrument_src_first_mod,\n",
     "    ctrl_src=ctrl_src,\n",
     "    raise_error=False,  # to be able to process very old data without gain_setting value\n",
     ")"
@@ -417,11 +416,22 @@
     "    acq_rate = agipd_cond.get_acq_rate()\n",
     "if mem_cells == -1:\n",
     "    mem_cells = agipd_cond.get_num_cells()\n",
+    "if mem_cells is None:\n",
+    "    # TODO FOR MR REVIEWERS:n This check is only useful in case we read mem_cells from fast data instead of slow data.\n",
+    "    # I vote for moving this ValueError to the function instead or we better return None and handle it from the Nb like now?\n",
+    "    raise ValueError(f\"No raw images found for {instrument_src_first_mod}\")\n",
     "# TODO: look for alternative for passing creation_time\n",
     "if gain_setting == -1:\n",
     "    gain_setting = agipd_cond.get_gain_setting(creation_time)\n",
     "if bias_voltage == -1:\n",
-    "    bias_voltage = agipd_cond.get_bias_voltage(karabo_id_control)\n",
+    "    # TODO FOR MR REVIEWERS: Shall I read all bias voltage modules and compare values\n",
+    "    # or could this be adding complications?\n",
+    "    # This comparison would work only in case there is an issue with the detector.\n",
+    "    # Then there is a possibility that the dark constants were injected with\n",
+    "    # different voltage values for each module. If this comparison is not done and this issue is in place\n",
+    "    # We will reach the same detector issue conclusion after realizing constants were not retrieved for all modules.\n",
+    "    bias_voltage = agipd_cond.get_bias_voltage(\n",
+    "        karabo_id_control, module=first_mod_channel)\n",
     "if integration_time == -1:\n",
     "    integration_time = agipd_cond.get_integration_time()\n",
     "if gain_mode == -1:\n",
@@ -436,9 +446,6 @@
    "metadata": {},
    "outputs": [],
    "source": [
-    "if mem_cells is None:\n",
-    "    raise ValueError(f\"No raw images found for {instrument_src_mod}\")\n",
-    "\n",
     "mem_cells_db = mem_cells if mem_cells_db == -1 else mem_cells_db\n",
     "\n",
     "print(f\"Maximum memory cells to calibrate: {mem_cells}\")"
diff --git a/notebooks/AGIPD/Characterize_AGIPD_Gain_Darks_NBC.ipynb b/notebooks/AGIPD/Characterize_AGIPD_Gain_Darks_NBC.ipynb
index bc519599e..48510efc7 100644
--- a/notebooks/AGIPD/Characterize_AGIPD_Gain_Darks_NBC.ipynb
+++ b/notebooks/AGIPD/Characterize_AGIPD_Gain_Darks_NBC.ipynb
@@ -245,12 +245,11 @@
    "source": [
     "step_timer.start()\n",
     "\n",
-    "# Read slow data from 1st channel only.\n",
-    "# Read all modules in one notebook and validate the conditions across detectors?\n",
-    "# Currently slurm jobs run per one module.\n",
+    "# Read slow data from 1st channel of the selected module only.\n",
+    "# Currently dark slurm jobs run per one module.\n",
+    "first_mod_channel = sorted(modules)[0]\n",
     "\n",
-    "# TODO: what if first module is not available. Maybe only channel 2 available\n",
-    "instrument_src_mod = instrument_src.format(modules[0])\n",
+    "instrument_src_mod = instrument_src.format(first_mod_channel)\n",
     "\n",
     "agipd_ctrl_dark = AgipdCtrlRuns(\n",
     "    raw_folder=in_folder,\n",
@@ -275,7 +274,8 @@
     "    acq_rate = agipd_ctrl_dark.get_acq_rate()\n",
     "\n",
     "if bias_voltage == 0:\n",
-    "    bias_voltage = agipd_ctrl_dark.get_bias_voltage(karabo_id_control)\n",
+    "    bias_voltage = agipd_ctrl_dark.get_bias_voltage(\n",
+    "        karabo_id_control, module=first_mod_channel)\n",
     "\n",
     "fixed_gain_mode = False\n",
     "if gain_mode == -1:\n",
diff --git a/src/cal_tools/agipdlib.py b/src/cal_tools/agipdlib.py
index 74c3b852b..f72b0136b 100644
--- a/src/cal_tools/agipdlib.py
+++ b/src/cal_tools/agipdlib.py
@@ -233,9 +233,11 @@ class AgipdCtrl:
         300 was the default bias_voltage value for
         MID_DET_AGIPD1M-1 and SPB_DET_AGIPD1M-1.
 
-        :param karabo_id_control: The karabo deviceId for the CONTROL device.
-        :param module: defaults to module 0
-        :return: bias voltage
+        Args:
+            karabo_id_control: The karabo deviceId for the CONTROL device.
+            module: defaults to module 0
+        Returns:
+            float: bias voltage value
         """
         # TODO: Add a breaking fix by passing the source and key through
         # get_bias_voltage arguments.
@@ -415,7 +417,11 @@ class AgipdCtrlRuns:
         self._validate_same_value("Integration Time", integration_times)
         return integration_times[0]
 
-    def get_bias_voltage(self, karabo_id_control: str = None):
+    def get_bias_voltage(
+        self,
+        karabo_id_control: str = None,
+        module: Optional[int] = 0
+    ):
         """
         Args:
             karabo_id_control (str):
@@ -425,7 +431,7 @@ class AgipdCtrlRuns:
             int: Bias voltage.
         """
         bias_voltages = [
-            c.get_bias_voltage(karabo_id_control) for c in self.run_ctrls]
+            c.get_bias_voltage(karabo_id_control, module) for c in self.run_ctrls]
         self._validate_same_value("Bias Voltage", bias_voltages)
         return bias_voltages[0]
 
-- 
GitLab