Repository: FaradayInstitution/BPX

Issue:
BPX parser raises irrelevant validation errors
In the presence of an invalid BPX object, the BPX parser reports non-existent errors in addition to real errors.

### Repeat

1. 
```
import bpx
bpx.parse_bpx_file("examples/nmc_pouch_cell_BPX.json", v_tol=1)
```

Here, the file is parsed with no errors (valid BPX).

2. **Delete** line 10 from `nmc_pouch_cell_BPX.json`:
```
"Ambient temperature [K]": 298.15,
```

3.
```
import bpx
bpx.parse_bpx_file("examples/nmc_pouch_cell_BPX.json", v_tol=1)
```

The parser raises 40 errors, most of which are not correctly identified (missing or extra fields). The only real error (missing required field `["Parameterisation"]["Cell"]["Ambient temperature [K]"]`) is raised twice, as the first and last reported error.

Discussion:
@ejfdickinson: @rtimms @martinjrobins @rtimms: Maybe related, it seems like some optional fields are not validated correctly. E.g. the below test fails (no `ValidationError` is raised even though title should be a string!)
```python3
    def test_bad_title(self):
        test = copy.copy(self.base)
        test["Header"]["Title"] = 1
        with self.assertRaises(ValidationError):
            parse_obj_as(BPX, test)
```

@Ubham16 can you take a look?

Solution patch:
diff --git a/bpx/schema.py b/bpx/schema.py
index 819146b..43d9eb7 100644
--- a/bpx/schema.py
+++ b/bpx/schema.py
@@ -1,9 +1,8 @@
 from __future__ import annotations
 
-from typing import Literal, Union, get_args
-from warnings import warn
+from typing import Any, Literal, Union, get_args
 
-from pydantic import BaseModel, ConfigDict, Field, model_validator
+from pydantic import BaseModel, ConfigDict, Field, ValidationError, field_validator, model_validator
 
 from bpx import Function, InterpolatedTable
 
@@ -381,10 +380,16 @@ class Parameterisation(ExtraBaseModel):
         alias="User-defined",
     )
 
+    @field_validator("negative_electrode", "positive_electrode", mode="before")
+    @classmethod
+    def _choose_electrode_type(cls, data: dict) -> dict:
+        if data.get("Particle"):
+            return ElectrodeBlended.model_validate(data)
+        return ElectrodeSingle.model_validate(data)
+
     @model_validator(mode="after")
     def _sto_limit_validation(self) -> Parameterisation:
-        check_sto_limits(self, self.__dict__)
-        return self
+        return check_sto_limits(self)
 
 
 class ParameterisationSPM(ExtraBaseModel):
@@ -408,10 +413,16 @@ class ParameterisationSPM(ExtraBaseModel):
         alias="User-defined",
     )
 
+    @field_validator("negative_electrode", "positive_electrode", mode="before")
+    @classmethod
+    def _choose_electrode_type(cls, data: dict) -> dict:
+        if data.get("Particle"):
+            return ElectrodeBlendedSPM.model_validate(data)
+        return ElectrodeSingleSPM.model_validate(data)
+
     @model_validator(mode="after")
     def _sto_limit_validation(self) -> ParameterisationSPM:
-        check_sto_limits(self, self.__dict__)
-        return self
+        return check_sto_limits(self)
 
 
 class BPX(ExtraBaseModel):
@@ -426,18 +437,32 @@ class BPX(ExtraBaseModel):
     parameterisation: Union[ParameterisationSPM, Parameterisation] = Field(alias="Parameterisation")
     validation: dict[str, Experiment] = Field(None, alias="Validation")
 
-    @model_validator(mode="after")
-    def model_based_validation(self) -> BPX:
-        model = self.header.model
-        parameter_class_name = self.parameterisation.__class__.__name__
-        allowed_combinations = [
-            ("Parameterisation", "DFN"),
-            ("Parameterisation", "SPMe"),
-            ("ParameterisationSPM", "SPM"),
-        ]
-        if (parameter_class_name, model) not in allowed_combinations:
-            warn(
-                f"The model type {model} does not correspond to the parameter set",
-                stacklevel=2,
-            )
-        return self
+    @model_validator(mode="before")
+    @classmethod
+    def _dispatch_param_subclasses(cls, data: Any) -> Any:  # noqa:ANN401
+        if isinstance(data, dict):
+            # validate header first, checks that the model type is valid
+            header = Header.model_validate(data.get("Header"))
+            model_type = header.model
+
+            # Choose the expected class based on model type
+            if model_type == "SPM":
+                expected_cls, fallback_cls = ParameterisationSPM, Parameterisation
+                error_msg = f"Valid parameter set does not correspond with the model type {model_type}"
+            else:
+                expected_cls, fallback_cls = Parameterisation, ParameterisationSPM
+                error_msg = f"Valid SPM parameter set does not correspond with the model type {model_type}"
+
+            try:
+                parameterisation = expected_cls.model_validate(data["Parameterisation"])
+            except ValidationError as e:
+                try:
+                    fallback_cls.model_validate(data["Parameterisation"])
+                    raise ValueError(error_msg) from e
+                except ValidationError:
+                    raise e from None
+
+            # return validated data to stop double validation
+            data["Header"] = header
+            data["Parameterisation"] = parameterisation
+        return data
diff --git a/bpx/validators.py b/bpx/validators.py
index 03cb8a4..43591fe 100644
--- a/bpx/validators.py
+++ b/bpx/validators.py
@@ -3,7 +3,7 @@ from warnings import warn
 from .base_extra_model import ExtraBaseModel
 
 
-def check_sto_limits(cls: ExtraBaseModel, values: dict) -> dict:
+def check_sto_limits(param: ExtraBaseModel) -> ExtraBaseModel:
     """
     Validates that the STO limits subbed into the OCPs give the correct voltage limits.
     Works if both OCPs are defined as functions.
@@ -12,21 +12,21 @@ def check_sto_limits(cls: ExtraBaseModel, values: dict) -> dict:
     """
 
     try:
-        ocp_n = values.get("negative_electrode").ocp.to_python_function()
-        ocp_p = values.get("positive_electrode").ocp.to_python_function()
+        ocp_n = param.negative_electrode.ocp.to_python_function()
+        ocp_p = param.positive_electrode.ocp.to_python_function()
     except AttributeError:
         # OCPs defined as interpolated tables or one of the electrodes is blended; do nothing
-        return values
+        return param
 
-    sto_n_min = values.get("negative_electrode").minimum_stoichiometry
-    sto_n_max = values.get("negative_electrode").maximum_stoichiometry
-    sto_p_min = values.get("positive_electrode").minimum_stoichiometry
-    sto_p_max = values.get("positive_electrode").maximum_stoichiometry
-    v_min = values.get("cell").lower_voltage_cutoff
-    v_max = values.get("cell").upper_voltage_cutoff
+    sto_n_min = param.negative_electrode.minimum_stoichiometry
+    sto_n_max = param.negative_electrode.maximum_stoichiometry
+    sto_p_min = param.positive_electrode.minimum_stoichiometry
+    sto_p_max = param.positive_electrode.maximum_stoichiometry
+    v_min = param.cell.lower_voltage_cutoff
+    v_max = param.cell.upper_voltage_cutoff
 
     # Voltage tolerance from `settings` data class
-    tol = cls.Settings.tolerances["Voltage [V]"]
+    tol = param.Settings.tolerances["Voltage [V]"]
 
     # Checks the maximum voltage estimated from STO
     v_max_sto = ocp_p(sto_p_min) - ocp_n(sto_n_max)
@@ -48,5 +48,4 @@ def check_sto_limits(cls: ExtraBaseModel, values: dict) -> dict:
             stacklevel=2,
         )
 
-    return values
-
+    return param


Test patch:
diff --git a/tests/test_schema.py b/tests/test_schema.py
index 98ab425..b7b2463 100644
--- a/tests/test_schema.py
+++ b/tests/test_schema.py
@@ -1,4 +1,5 @@
 import copy
+import re
 import unittest
 import warnings
 from typing import Any
@@ -213,37 +214,34 @@ class TestSchema(unittest.TestCase):
         test = copy.deepcopy(self.base_spm)
         adapter.validate_python(test)
 
+    def test_missing_model(self) -> None:
+        test = copy.deepcopy(self.base)
+        del test["Header"]["Model"]
+        with pytest.raises(ValidationError, match=re.escape("Model\n  Field required")):
+            adapter.validate_python(test)
+
     def test_bad_model(self) -> None:
         test = copy.deepcopy(self.base)
         test["Header"]["Model"] = "Wrong model type"
-        with pytest.raises(ValidationError):
+        with pytest.raises(ValidationError, match="Input should be 'SPM', 'SPMe' or 'DFN'"):
             adapter.validate_python(test)
 
     def test_bad_dfn(self) -> None:
         test = copy.deepcopy(self.base_spm)
         test["Header"]["Model"] = "DFN"
-        with pytest.warns(
-            UserWarning,
-            match="The model type DFN does not correspond to the parameter set",
-        ):
+        with pytest.raises(ValueError, match="Valid SPM parameter set does not correspond with"):
             adapter.validate_python(test)
 
     def test_bad_spme(self) -> None:
         test = copy.deepcopy(self.base_spm)
         test["Header"]["Model"] = "SPMe"
-        with pytest.warns(
-            UserWarning,
-            match="The model type SPMe does not correspond to the parameter set",
-        ):
+        with pytest.raises(ValueError, match="Valid SPM parameter set does not correspond with"):
             adapter.validate_python(test)
 
     def test_bad_spm(self) -> None:
         test = copy.deepcopy(self.base)
         test["Header"]["Model"] = "SPM"
-        with pytest.warns(
-            UserWarning,
-            match="The model type SPM does not correspond to the parameter set",
-        ):
+        with pytest.raises(ValueError, match="Valid parameter set does not correspond with"):
             adapter.validate_python(test)
 
     def test_table(self) -> None:
@@ -293,9 +291,16 @@ class TestSchema(unittest.TestCase):
     def test_bad_input(self) -> None:
         test = copy.deepcopy(self.base)
         test["Parameterisation"]["Electrolyte"]["bad"] = "this shouldn't be here"
-        with pytest.raises(ValidationError):
+        with pytest.raises(
+            ValidationError,
+            match=re.escape("Electrolyte.bad\n  Extra inputs are not permitted"),
+        ) as excinfo:
             adapter.validate_python(test)
 
+        # checks that only the relevent error is raised, not a cascade of unreleated errors
+        errors = excinfo.value.errors()
+        assert len(errors) == 1, f"Expected 1 error, got {len(errors)}: {errors}"
+
     def test_validation_data(self) -> None:
         test = copy.deepcopy(self.base)
         test["Validation"] = {
@@ -312,6 +317,7 @@ class TestSchema(unittest.TestCase):
                 "Temperature [K]": [298.15, 298.15],
             },
         }
+        adapter.validate_python(test)
 
     def test_check_sto_limits_validator(self) -> None:
         warnings.filterwarnings("error")  # Treat warnings as errors