Merge pull request #115 from CCPBioSim/83-sanity-checks-for-input-parameters

Sanity Checks for Input Parameters

Author: harryswift01

CodeEntropy/config/arg_config_manager.py

@@ -85,15 +85,54 @@ def load_config(self, file_path):
 
         return config
 
+    def str2bool(self, value):
+        """
+        Convert a string or boolean input into a boolean value.
+
+        Accepts common string representations of boolean values such as:
+        - True values: "true", "t", "yes", "1"
+        - False values: "false", "f", "no", "0"
+
+        If the input is already a boolean, it is returned as-is.
+        Raises:
+            argparse.ArgumentTypeError: If the input cannot be interpreted as a boolean.
+
+        Args:
+            value (str or bool): The input value to convert.
+
+        Returns:
+            bool: The corresponding boolean value.
+        """
+        if isinstance(value, bool):
+            return value
+        value = value.lower()
+        if value in {"true", "t", "yes", "1"}:
+            return True
+        elif value in {"false", "f", "no", "0"}:
+            return False
+        else:
+            raise argparse.ArgumentTypeError("Boolean value expected (True/False).")
+
     def setup_argparse(self):
         """Setup argument parsing dynamically based on arg_map."""
         parser = argparse.ArgumentParser(
             description="CodeEntropy: Entropy calculation with MCC method."
         )
 
         for arg, properties in self.arg_map.items():
-            kwargs = {key: properties[key] for key in properties if key != "help"}
-            parser.add_argument(f"--{arg}", **kwargs, help=properties.get("help"))
+            help_text = properties.get("help", "")
+            default = properties.get("default", None)
+
+            if properties.get("type") == bool:
+                parser.add_argument(
+                    f"--{arg}",
+                    type=self.str2bool,
+                    default=default,
+                    help=f"{help_text} (default: {default})",
+                )
+            else:
+                kwargs = {k: v for k, v in properties.items() if k != "help"}
+                parser.add_argument(f"--{arg}", **kwargs, help=help_text)
 
         return parser
 
@@ -146,3 +185,62 @@ def merge_configs(self, args, run_config):
                 handler.setLevel(logging.INFO)
 
         return args
+
+    def input_parameters_validation(self, u, args):
+        """Check the validity of the user inputs against sensible values"""
+
+        self._check_input_start(u, args)
+        self._check_input_end(u, args)
+        self._check_input_step(args)
+        self._check_input_bin_width(args)
+        self._check_input_temperature(args)
+        self._check_input_force_partitioning(args)
+
+    def _check_input_start(self, u, args):
+        """Check that the input does not exceed the length of the trajectory."""
+        if args.start > len(u.trajectory):
+            raise ValueError(
+                f"Invalid 'start' value: {args.start}. It exceeds the trajectory length"
+                " of {len(u.trajectory)}."
+            )
+
+    def _check_input_end(self, u, args):
+        """Check that the end index does not exceed the trajectory length."""
+        if args.end > len(u.trajectory):
+            raise ValueError(
+                f"Invalid 'end' value: {args.end}. It exceeds the trajectory length of"
+                " {len(u.trajectory)}."
+            )
+
+    def _check_input_step(self, args):
+        """Check that the step value is non-negative."""
+        if args.step < 0:
+            logger.warning(
+                f"Negative 'step' value provided: {args.step}. This may lead to"
+                " unexpected behavior."
+            )
+
+    def _check_input_bin_width(self, args):
+        """Check that the bin width is within the valid range [0, 360]."""
+        if args.bin_width < 0 or args.bin_width > 360:
+            raise ValueError(
+                f"Invalid 'bin_width': {args.bin_width}. It must be between 0 and 360"
+                " degrees."
+            )
+
+    def _check_input_temperature(self, args):
+        """Check that the temperature is non-negative."""
+        if args.temperature < 0:
+            raise ValueError(
+                f"Invalid 'temperature': {args.temperature}. Temperature cannot be"
+                " below 0."
+            )
+
+    def _check_input_force_partitioning(self, args):
+        """Warn if force partitioning is not set to the default value."""
+        default_value = arg_map["force_partitioning"]["default"]
+        if args.force_partitioning != default_value:
+            logger.warning(
+                f"'force_partitioning' is set to {args.force_partitioning},"
+                " which differs from the default ({default_value})."
+            )

CodeEntropy/run.py

@@ -127,14 +127,16 @@ def run_entropy_workflow(self):
                 # Log all inputs for the current run
                 logger.info(f"All input for {run_name}")
                 for arg in vars(args):
-                    logger.info(f" {arg}: {getattr(args, arg) or ''}")
+                    logger.info(f" {arg}: {getattr(args, arg)}")
 
                 # Load MDAnalysis Universe
                 tprfile = args.top_traj_file[0]
                 trrfile = args.top_traj_file[1:]
                 logger.debug(f"Loading Universe with {tprfile} and {trrfile}")
                 u = mda.Universe(tprfile, trrfile)
 
+                self._config_manager.input_parameters_validation(u, args)
+
                 # Create LevelManager instance
                 level_manager = LevelManager()
 

config.yaml

@@ -12,4 +12,4 @@ run1:
   thread:
   output_file:
   force_partitioning:
-  disable_water_entropy:
+  water_entropy:

tests/test_CodeEntropy/test_arg_config_manager.py

@@ -137,6 +137,77 @@ def test_setup_argparse(self, mock_args):
         self.assertEqual(args.top_traj_file, ["/path/to/tpr", "/path/to/trr"])
         self.assertEqual(args.selection_string, "all")
 
+    @patch(
+        "argparse.ArgumentParser.parse_args",
+        return_value=MagicMock(
+            top_traj_file=["/path/to/tpr", "/path/to/trr"],
+            start=10,
+            water_entropy=False,
+        ),
+    )
+    def test_setup_argparse_false_boolean(self, mock_args):
+        """
+        Test that non-boolean arguments are parsed correctly.
+        """
+        arg_config = ConfigManager()
+        parser = arg_config.setup_argparse()
+        args = parser.parse_args()
+
+        self.assertEqual(args.top_traj_file, ["/path/to/tpr", "/path/to/trr"])
+        self.assertEqual(args.start, 10)
+        self.assertFalse(args.water_entropy)
+
+    def test_str2bool_true_variants(self):
+        """Test that various string representations of True are correctly parsed."""
+        arg_config = ConfigManager()
+
+        self.assertTrue(arg_config.str2bool("true"))
+        self.assertTrue(arg_config.str2bool("True"))
+        self.assertTrue(arg_config.str2bool("t"))
+        self.assertTrue(arg_config.str2bool("yes"))
+        self.assertTrue(arg_config.str2bool("1"))
+
+    def test_str2bool_false_variants(self):
+        """Test that various string representations of False are correctly parsed."""
+        arg_config = ConfigManager()
+
+        self.assertFalse(arg_config.str2bool("false"))
+        self.assertFalse(arg_config.str2bool("False"))
+        self.assertFalse(arg_config.str2bool("f"))
+        self.assertFalse(arg_config.str2bool("no"))
+        self.assertFalse(arg_config.str2bool("0"))
+
+    def test_str2bool_boolean_passthrough(self):
+        """Test that boolean values passed directly are returned unchanged."""
+        arg_config = ConfigManager()
+
+        self.assertTrue(arg_config.str2bool(True))
+        self.assertFalse(arg_config.str2bool(False))
+
+    def test_str2bool_invalid_input(self):
+        """Test that invalid string inputs raise an ArgumentTypeError."""
+        arg_config = ConfigManager()
+
+        with self.assertRaises(Exception) as context:
+            arg_config.str2bool("maybe")
+        self.assertIn("Boolean value expected", str(context.exception))
+
+    def test_str2bool_empty_string(self):
+        """Test that an empty string raises an ArgumentTypeError."""
+        arg_config = ConfigManager()
+
+        with self.assertRaises(Exception) as context:
+            arg_config.str2bool("")
+        self.assertIn("Boolean value expected", str(context.exception))
+
+    def test_str2bool_unexpected_number(self):
+        """Test that unexpected numeric strings raise an ArgumentTypeError."""
+        arg_config = ConfigManager()
+
+        with self.assertRaises(Exception) as context:
+            arg_config.str2bool("2")
+        self.assertIn("Boolean value expected", str(context.exception))
+
     def test_cli_overrides_defaults(self):
         """
         Test if CLI parameters override default values.
@@ -423,6 +494,103 @@ def test_empty_yaml_config(self, mock_exists, mock_file):
         self.assertIsInstance(config, dict)
         self.assertEqual(config, {})
 
+    def test_input_parameters_validation_all_valid(self):
+        """Test that input_parameters_validation passes with all valid inputs."""
+        manager = ConfigManager()
+        u = MagicMock()
+        u.trajectory = [0] * 100
+
+        args = MagicMock(
+            start=10,
+            end=90,
+            step=1,
+            bin_width=30,
+            temperature=298.0,
+            force_partitioning=0.5,
+        )
+
+        with patch.dict(
+            "CodeEntropy.config.arg_config_manager.arg_map",
+            {"force_partitioning": {"default": 0.5}},
+        ):
+            manager.input_parameters_validation(u, args)
+
+    def test_check_input_start_valid(self):
+        """Test that a valid 'start' value does not raise an error."""
+        args = MagicMock(start=50)
+        u = MagicMock()
+        u.trajectory = [0] * 100
+        ConfigManager()._check_input_start(u, args)
+
+    def test_check_input_start_invalid(self):
+        """Test that an invalid 'start' value raises a ValueError."""
+        args = MagicMock(start=150)
+        u = MagicMock()
+        u.trajectory = [0] * 100
+        with self.assertRaises(ValueError):
+            ConfigManager()._check_input_start(u, args)
+
+    def test_check_input_end_valid(self):
+        """Test that a valid 'end' value does not raise an error."""
+        args = MagicMock(end=100)
+        u = MagicMock()
+        u.trajectory = [0] * 100
+        ConfigManager()._check_input_end(u, args)
+
+    def test_check_input_end_invalid(self):
+        """Test that an 'end' value exceeding trajectory length raises a ValueError."""
+        args = MagicMock(end=101)
+        u = MagicMock()
+        u.trajectory = [0] * 100
+        with self.assertRaises(ValueError):
+            ConfigManager()._check_input_end(u, args)
+
+    @patch("CodeEntropy.config.arg_config_manager.logger")
+    def test_check_input_step_negative(self, mock_logger):
+        """Test that a negative 'step' value triggers a warning."""
+        args = MagicMock(step=-1)
+        ConfigManager()._check_input_step(args)
+        mock_logger.warning.assert_called_once()
+
+    def test_check_input_bin_width_valid(self):
+        """Test that a valid 'bin_width' value does not raise an error."""
+        args = MagicMock(bin_width=180)
+        ConfigManager()._check_input_bin_width(args)
+
+    def test_check_input_bin_width_invalid_low(self):
+        """Test that a negative 'bin_width' value raises a ValueError."""
+        args = MagicMock(bin_width=-10)
+        with self.assertRaises(ValueError):
+            ConfigManager()._check_input_bin_width(args)
+
+    def test_check_input_bin_width_invalid_high(self):
+        """Test that a 'bin_width' value above 360 raises a ValueError."""
+        args = MagicMock(bin_width=400)
+        with self.assertRaises(ValueError):
+            ConfigManager()._check_input_bin_width(args)
+
+    def test_check_input_temperature_valid(self):
+        """Test that a valid 'temperature' value does not raise an error."""
+        args = MagicMock(temperature=298.0)
+        ConfigManager()._check_input_temperature(args)
+
+    def test_check_input_temperature_invalid(self):
+        """Test that a negative 'temperature' value raises a ValueError."""
+        args = MagicMock(temperature=-5)
+        with self.assertRaises(ValueError):
+            ConfigManager()._check_input_temperature(args)
+
+    @patch("CodeEntropy.config.arg_config_manager.logger")
+    def test_check_input_force_partitioning_warning(self, mock_logger):
+        """Test that a non-default 'force_partitioning' value triggers a warning."""
+        args = MagicMock(force_partitioning=0.7)
+        with patch.dict(
+            "CodeEntropy.config.arg_config_manager.arg_map",
+            {"force_partitioning": {"default": 0.5}},
+        ):
+            ConfigManager()._check_input_force_partitioning(args)
+            mock_logger.warning.assert_called_once()
+
 
 if __name__ == "__main__":
     unittest.main()