Fix parameter set logic and allow FittingProblem to take init_ocv rather than init_soc

pybop/models/base_model.py

@@ -195,13 +195,13 @@
             If the input is not a dictionary with a single, valid key.
         """
         if len(initial_state) > 1:
             raise ValueError("Expecting only one initial state.")
         elif "Initial SoC" in initial_state.keys():
             return initial_state["Initial SoC"]
         elif "Initial open-circuit voltage [V]" in initial_state.keys():
             return str(initial_state["Initial open-circuit voltage [V]"]) + "V"
         else:
             raise ValueError("Unrecognised initial state.")
 
     def set_initial_state(self, initial_state: dict):
         """
@@ -424,9 +424,9 @@
 
         Returns
         -------
-        pybamm.Solution or [np.inf]
-            The solution object returned by a PyBaMM simulation, or [np.inf] in the case where
-            the parameter values are infeasible and infeasible solutions are not allowed.
+        pybamm.Solution
+            The solution object returned by a PyBaMM simulation, or a pybamm error in the case
+            where the parameter values are infeasible and infeasible solutions are not allowed.
 
         Raises
         ------
@@ -452,19 +452,13 @@
             self.parameters.update(values=list(inputs.values()))
             self.rebuild(parameters=self.parameters, initial_state=initial_state)
 
-        if self.check_params(
+        if not self.check_params(
             inputs=inputs,
             allow_infeasible_solutions=self.allow_infeasible_solutions,
         ):
-            try:
-                return self.solver.solve(
-                    self._built_model, inputs=inputs, t_eval=t_eval
-                )
-            except Exception as e:
-                print(f"Error: {e}")
-                return [np.inf]
-        else:
-            return [np.inf]
+            raise ValueError("These parameter values are infeasible.")
+
+        return self.solver.solve(self._built_model, inputs=inputs, t_eval=t_eval)
 
     def simulateS1(
         self, inputs: Inputs, t_eval: np.array, initial_state: Optional[dict] = None
@@ -486,9 +480,9 @@
 
         Returns
         -------
-        pybamm.Solution or [np.inf]
-            The solution object returned by a PyBaMM simulation, or [np.inf] in the case where
-            the parameter values are infeasible and infeasible solutions are not allowed.
+        pybamm.Solution
+            The solution object returned by a PyBaMM simulation, or a pybamm error in the case
+            where the parameter values are infeasible and infeasible solutions are not allowed.
 
         Raises
         ------
@@ -505,23 +499,18 @@
                 "Cannot use sensitivies for parameters which require a model rebuild"
             )
 
-        if self.check_params(
+        if not self.check_params(
             inputs=inputs,
             allow_infeasible_solutions=self.allow_infeasible_solutions,
         ):
-            try:
-                return self._solver.solve(
-                    self._built_model,
-                    inputs=inputs,
-                    t_eval=t_eval,
-                    calculate_sensitivities=True,
-                )
-
-            except Exception as e:
-                print(f"Error: {e}")
-                return [np.inf]
-        else:
-            return [np.inf]
+            raise ValueError("These parameter values are infeasible.")
+
+        return self._solver.solve(
+            self._built_model,
+            inputs=inputs,
+            t_eval=t_eval,
+            calculate_sensitivities=True,
+        )
 
     def predict(
         self,
@@ -559,9 +548,9 @@
 
         Returns
         -------
-        pybamm.Solution or [np.inf]
-            The solution object returned by a PyBaMM simulation, or [np.inf] in the case where
-            the parameter values are infeasible and infeasible solutions are not allowed.
+        pybamm.Solution
+            The solution object returned by a PyBaMM simulation, or a pybamm error in the case
+            where the parameter values are infeasible and infeasible solutions are not allowed.
 
         Raises
         ------
@@ -591,29 +580,28 @@
                 parameter_set["Initial SoC"] = self._parameter_set["Initial SoC"]
                 initial_state = None
 
-        if self.check_params(
+        if not self.check_params(
             parameter_set=parameter_set,
             allow_infeasible_solutions=self.allow_infeasible_solutions,
         ):
-            if experiment is not None:
-                return pybamm.Simulation(
-                    model=self._unprocessed_model,
-                    experiment=experiment,
-                    parameter_values=parameter_set,
-                ).solve(initial_soc=initial_state)
-            elif t_eval is not None:
-                return pybamm.Simulation(
-                    model=self._unprocessed_model,
-                    parameter_values=parameter_set,
-                ).solve(t_eval=t_eval, initial_soc=initial_state)
-            else:
-                raise ValueError(
-                    "The predict method requires either an experiment or "
-                    "t_eval to be specified."
-                )
-
+            raise ValueError("These parameter values are infeasible.")
+
+        if experiment is not None:
+            return pybamm.Simulation(
+                model=self._unprocessed_model,
+                experiment=experiment,
+                parameter_values=parameter_set,
+            ).solve(initial_soc=initial_state)
+        elif t_eval is not None:
+            return pybamm.Simulation(
+                model=self._unprocessed_model,
+                parameter_values=parameter_set,
+            ).solve(t_eval=t_eval, initial_soc=initial_state)
         else:
-            return [np.inf]
+            raise ValueError(
+                "The predict method requires either an experiment or t_eval "
+                "to be specified."
+            )
 
     def check_params(
         self,

pybop/problems/fitting_problem.py

@@ -104,18 +104,19 @@
         """
         inputs = self.parameters.verify(inputs)
 
-        sol = self._model.simulate(
-            inputs=inputs, t_eval=self._time_data, initial_state=self.initial_state
-        )
-
-        if sol == [np.inf]:
+        try:
+            sol = self._model.simulate(
+                inputs=inputs, t_eval=self._time_data, initial_state=self.initial_state
+            )
+        except Exception as e:
+            if self.verbose:
+                print(f"Simulation error: {e}")
             return {signal: [np.inf] for signal in self.signal}
 
-        else:
-            return {
-                signal: sol[signal].data
-                for signal in (self.signal + self.additional_variables)
-            }
+        return {
+            signal: sol[signal].data
+            for signal in (self.signal + self.additional_variables)
+        }
 
     def evaluateS1(self, inputs: Inputs):
         """
@@ -134,32 +135,32 @@
         """
         inputs = self.parameters.verify(inputs)
 
-        sol = self._model.simulateS1(
-            inputs=inputs, t_eval=self._time_data, initial_state=self.initial_state
-        )
-
-        if sol == [np.inf]:
+        try:
+            sol = self._model.simulateS1(
+                inputs=inputs, t_eval=self._time_data, initial_state=self.initial_state
+            )
+        except Exception as e:
+            print(f"Error: {e}")
             return {signal: [np.inf] for signal in self.signal}, [np.inf]
 
-        else:
-            y = {signal: sol[signal].data for signal in self.signal}
+        y = {signal: sol[signal].data for signal in self.signal}
 
-            # Extract the sensitivities and stack them along a new axis for each signal
-            dy = np.empty(
-                (
-                    sol[self.signal[0]].data.shape[0],
-                    self.n_outputs,
-                    self._n_parameters,
-                )
+        # Extract the sensitivities and stack them along a new axis for each signal
+        dy = np.empty(
+            (
+                sol[self.signal[0]].data.shape[0],
+                self.n_outputs,
+                self._n_parameters,
             )
+        )
 
-            for i, signal in enumerate(self.signal):
-                dy[:, i, :] = np.stack(
-                    [
-                        sol[signal].sensitivities[key].toarray()[:, 0]
-                        for key in self.parameters.keys()
-                    ],
-                    axis=-1,
-                )
+        for i, signal in enumerate(self.signal):
+            dy[:, i, :] = np.stack(
+                [
+                    sol[signal].sensitivities[key].toarray()[:, 0]
+                    for key in self.parameters.keys()
+                ],
+                axis=-1,
+            )
 
-            return (y, np.asarray(dy))
+        return (y, np.asarray(dy))

tests/unit/test_models.py

@@ -135,8 +135,10 @@ def test_predict_without_allow_infeasible_solutions(self, model):
                 "Positive electrode active material volume fraction": 0.9,
             }
 
-            res = model.predict(t_eval=t_eval, inputs=inputs)
-            assert np.isinf(res).any()
+            with pytest.raises(
+                ValueError, match="These parameter values are infeasible."
+            ):
+                model.predict(t_eval=t_eval, inputs=inputs)
 
     @pytest.mark.unit
     def test_build(self, model):