From f6a80ecfc40a4ed87f2e2869045d988efae3817e Mon Sep 17 00:00:00 2001
From: JosePizarro3 <jose.pizarro@physik.hu-berlin.de>
Date: Mon, 8 Apr 2024 10:21:24 +0200
Subject: [PATCH] Added normalization for Variables.n_bins

General fixes after Joe's comments
---
 src/nomad_simulations/outputs.py           |  8 ++--
 src/nomad_simulations/physical_property.py | 11 +----
 src/nomad_simulations/variables.py         | 53 +++++++++++++++++++---
 3 files changed, 53 insertions(+), 19 deletions(-)

diff --git a/src/nomad_simulations/outputs.py b/src/nomad_simulations/outputs.py
index 429fbc27..ca6cd894 100644
--- a/src/nomad_simulations/outputs.py
+++ b/src/nomad_simulations/outputs.py
@@ -29,7 +29,7 @@
 from .numerical_settings import SelfConsistency
 from .physical_property import PhysicalProperty
 
-from .variables import Variables, Temperatures  # ? delete these imports
+from .variables import Variables, Temperature  # ? delete these imports
 
 
 class ElectronicBandGap(PhysicalProperty):
@@ -80,9 +80,9 @@ class Outputs(ArchiveSection):
         a_eln=ELNAnnotation(component='ReferenceEditQuantity'),
     )
 
-    # # # # # # # # # #
+    # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
     # List of properties
-
+    # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
     electronic_band_gap = SubSection(sub_section=ElectronicBandGap.m_def, repeats=True)
 
     def normalize(self, archive, logger) -> None:
@@ -137,7 +137,7 @@ def normalize(self, archive, logger) -> None:
 # Playing with `PhysicalProperty`
 band_gap = ElectronicBandGap(source='simulation', type='direct', label='DFT')
 n_bins = 3
-temperature = Temperatures(n_bins=n_bins, bins=np.linspace(0, 100, n_bins))
+temperature = Temperature(n_bins=n_bins, bins=np.linspace(0, 100, n_bins))
 band_gap.variables.append(temperature)
 n_bins = 2
 custom_bins = Variables(n_bins=n_bins, bins=np.linspace(0, 100, n_bins))
diff --git a/src/nomad_simulations/physical_property.py b/src/nomad_simulations/physical_property.py
index 0852d17b..c96eaaee 100644
--- a/src/nomad_simulations/physical_property.py
+++ b/src/nomad_simulations/physical_property.py
@@ -77,16 +77,9 @@ class PhysicalProperty(ArchiveSection):
         """,
     )
 
-    variables = SubSection(
-        type=Variables.m_def,
-        description="""
-        Variables over which the physical property varies. These are defined as binned, i.e., discretized
-        values by `n_bins` and `bins`. The `variables` are used to calculate the `variables_shape` of the physical property.
-        """,
-        repeats=True,
-    )
+    variables = SubSection(type=Variables.m_def, repeats=True)
 
-    # ! this is not working for now, because I want to m_set the values of `n_bins` and `bins` like `MSection` has implemented
+    # ! When having an underlying structure, the `DataType` inheritance is not enough, as we want to anyways `m_set` the quantities defined in `Variables` (`bins`, `bins_errors`)
     # variables = Quantity(
     #     type=Variables,
     #     shape=['*'],
diff --git a/src/nomad_simulations/variables.py b/src/nomad_simulations/variables.py
index 08c29436..38694b7c 100644
--- a/src/nomad_simulations/variables.py
+++ b/src/nomad_simulations/variables.py
@@ -23,24 +23,65 @@
 
 
 class Variables(ArchiveSection):
-    """ """
+    """
+    Variables over which the physical property varies. These are defined as binned, i.e., discretized
+    values by `n_bins` and `bins`. These are used to calculate the `shape` of the physical property.
+    """
+
+    name = Quantity(
+        type=str,
+        default='Custom',
+        description="""
+        Name of the variable.
+        """,
+    )
+
+    n_bins = Quantity(
+        type=int,
+        description="""
+        Number of bins.
+        """,
+    )
+
+    bins = Quantity(
+        type=np.float64,
+        shape=['n_bins'],
+        description="""
+        Bins of the variable.
+        """,
+    )
 
-    name = Quantity(type=str, default='custom')
-    n_bins = Quantity(type=int)
-    bins = Quantity(type=np.float64, shape=['n_bins'])
     # bins_error = Quantity()
 
     def normalize(self, archive, logger) -> None:
         super().normalize(archive, logger)
 
+        # Setting `n_bins` if these are not defined
+        if self.bins is not None:
+            if self.n_bins != len(self.bins):
+                logger.warning(
+                    f'The stored `n_bins`, {self.n_bins}, does not coincide with the length of `bins`, {len(self.bins)}. We will re-assign `n_bins` as the length of `bins`.'
+                )
+            self.n_bins = len(self.bins)
+
+
+class Temperature(Variables):
+    """ """
 
-class Temperatures(Variables):
     def __init__(self, m_def: Section = None, m_context: Context = None, **kwargs):
         super().__init__(m_def, m_context, **kwargs)
         self.name = self.m_def.name
 
+    def normalize(self, archive, logger) -> None:
+        super().normalize(archive, logger)
+
+
+class Energy(Variables):
+    """ """
 
-class Energies(Variables):
     def __init__(self, m_def: Section = None, m_context: Context = None, **kwargs):
         super().__init__(m_def, m_context, **kwargs)
         self.name = self.m_def.name
+
+    def normalize(self, archive, logger) -> None:
+        super().normalize(archive, logger)