Skip to content

Commit

Permalink
Merge pull request #2276 from pybamm-team/issue-2275-remove-processed…
Browse files Browse the repository at this point in the history
…-symbolic-variable

#2275 remove ProcessedSymbolicVariable
  • Loading branch information
valentinsulzer authored Sep 6, 2022
2 parents 6142acc + f550813 commit cd640a5
Show file tree
Hide file tree
Showing 12 changed files with 112 additions and 708 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ example notebook ([#1602](https://github.com/pybamm-team/PyBaMM/pull/1602))
## Breaking changes

- Refactored the `particle` submodel module, with the models having no size distribution now found in `particle.no_distribution`, and those with a size distribution in `particle.size_distribution`. Renamed submodels to indicate the transport model (Fickian diffusion, polynomial profile) and if they are "x-averaged". E.g., `FickianManyParticles` and `FickianSingleParticle` are now `no_distribution.FickianDiffusion` and `no_distribution.XAveragedFickianDiffusion` ([#1602](https://github.com/pybamm-team/PyBaMM/pull/1602))
- Changed sensitivity API. Removed `ProcessedSymbolicVariable`, all sensitivity now handled within the solvers and `ProcessedVariable` () ([#1552](https://github.com/pybamm-team/PyBaMM/pull/1552))
- Changed sensitivity API. Removed `ProcessedSymbolicVariable`, all sensitivity now handled within the solvers and `ProcessedVariable` ([#1552](https://github.com/pybamm-team/PyBaMM/pull/1552),[#2276](https://github.com/pybamm-team/PyBaMM/pull/2276))
- The `Yang2017` parameter set has been removed as the complete parameter set is not publicly available in the literature ([#1577](https://github.com/pybamm-team/PyBaMM/pull/1577))
- Changed how options are specified for the "loss of active material" and "particle cracking" submodels. "loss of active material" can now be one of "none", "stress-driven", or "reaction-driven", or a 2-tuple for different options in negative and positive electrode. Similarly "particle cracking" (now called "particle mechanics") can now be "none", "swelling only", "swelling and cracking", or a 2-tuple ([#1490](https://github.com/pybamm-team/PyBaMM/pull/1490))
- Changed the variable in the full diffusion model from "Electrolyte concentration" to "Porosity times concentration" ([#1476](https://github.com/pybamm-team/PyBaMM/pull/1476))
Expand Down
1 change: 0 additions & 1 deletion pybamm/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@
#
from .solvers.solution import Solution, make_cycle_solution
from .solvers.processed_variable import ProcessedVariable
from .solvers.processed_symbolic_variable import ProcessedSymbolicVariable
from .solvers.base_solver import BaseSolver
from .solvers.dummy_solver import DummySolver
from .solvers.algebraic_solver import AlgebraicSolver
Expand Down
13 changes: 2 additions & 11 deletions pybamm/solvers/base_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -1481,20 +1481,11 @@ def _set_up_ext_and_inputs(
inputs = inputs or {}

# Go through all input parameters that can be found in the model
# If any of them are *not* provided by "inputs", a symbolic input parameter is
# created, with appropriate size
# If any of them are *not* provided by "inputs", raise an error
for input_param in model.input_parameters:
name = input_param.name
if name not in inputs:
# Only allow symbolic inputs for CasadiSolver and CasadiAlgebraicSolver
if not isinstance(
self, (pybamm.CasadiSolver, pybamm.CasadiAlgebraicSolver)
):
raise pybamm.SolverError(
"Only CasadiSolver and CasadiAlgebraicSolver "
"can have symbolic inputs"
)
inputs[name] = casadi.MX.sym(name, input_param._expected_size)
raise pybamm.SolverError(f"No value provided for input '{name}'")

external_variables = external_variables or {}

Expand Down
20 changes: 4 additions & 16 deletions pybamm/solvers/casadi_algebraic_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,10 @@ def _integrate(self, model, t_eval, inputs_dict=None):
t_eval : :class:`numpy.array`, size (k,)
The times at which to compute the solution
inputs_dict : dict, optional
Any input parameters to pass to the model when solving. If any input
parameters that are present in the model are missing from "inputs", then
the solution will consist of `ProcessedSymbolicVariable` objects, which must
be provided with inputs to obtain their value.
Any input parameters to pass to the model when solving.
"""
# Record whether there are any symbolic inputs
inputs_dict = inputs_dict or {}
has_symbolic_inputs = any(
isinstance(v, casadi.MX) for v in inputs_dict.values()
)
symbolic_inputs = casadi.vertcat(
*[v for v in inputs_dict.values() if isinstance(v, casadi.MX)]
)

# Create casadi objects for the root-finder
inputs = casadi.vertcat(*[v for v in inputs_dict.values()])
Expand Down Expand Up @@ -94,7 +85,6 @@ def _integrate(self, model, t_eval, inputs_dict=None):
y_alg_sym = casadi.MX.sym("y_alg", y0_alg.shape[0])
y_sym = casadi.vertcat(y0_diff, y_alg_sym)

t_and_inputs_sym = casadi.vertcat(t_sym, symbolic_inputs)
alg = model.casadi_algebraic(t_sym, y_sym, inputs)

# Check interpolant extrapolation
Expand Down Expand Up @@ -139,7 +129,7 @@ def _integrate(self, model, t_eval, inputs_dict=None):
roots = casadi.rootfinder(
"roots",
"newton",
dict(x=y_alg_sym, p=t_and_inputs_sym, g=alg),
dict(x=y_alg_sym, p=t_sym, g=alg),
{
**self.extra_options,
"abstol": self.tol,
Expand All @@ -150,11 +140,10 @@ def _integrate(self, model, t_eval, inputs_dict=None):
timer = pybamm.Timer()
integration_time = 0
for idx, t in enumerate(t_eval):
t_eval_inputs_sym = casadi.vertcat(t, symbolic_inputs)
# Solve
try:
timer.reset()
y_alg_sol = roots(y0_alg, t_eval_inputs_sym)
y_alg_sol = roots(y0_alg, t)
integration_time += timer.time()
success = True
message = None
Expand All @@ -169,8 +158,7 @@ def _integrate(self, model, t_eval, inputs_dict=None):
# If there are no symbolic inputs, check the function is below the tol
# Skip this check if there are symbolic inputs
if success and (
has_symbolic_inputs is True
or (not any(np.isnan(fun)) and np.all(casadi.fabs(fun) < self.tol))
(not any(np.isnan(fun)) and np.all(casadi.fabs(fun) < self.tol))
):
# update initial guess for the next iteration
y0_alg = y_alg_sol
Expand Down
21 changes: 1 addition & 20 deletions pybamm/solvers/casadi_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,19 +125,12 @@ def _integrate(self, model, t_eval, inputs_dict=None):

# Record whether there are any symbolic inputs
inputs_dict = inputs_dict or {}
has_symbolic_inputs = any(
isinstance(v, casadi.MX) for v in inputs_dict.values()
)

# convert inputs to casadi format
inputs = casadi.vertcat(*[x for x in inputs_dict.values()])

# Calculate initial event signs needed for some of the modes
if (
has_symbolic_inputs is False
and self.mode != "fast"
and model.terminate_events_eval
):
if self.mode != "fast" and model.terminate_events_eval:
init_event_signs = np.sign(
np.concatenate(
[
Expand All @@ -149,18 +142,6 @@ def _integrate(self, model, t_eval, inputs_dict=None):
else:
init_event_signs = np.sign([])

if has_symbolic_inputs:
# Create integrator without grid to avoid having to create several times
self.create_integrator(model, inputs)
solution = self._run_integrator(
model,
model.y0,
inputs_dict,
inputs,
t_eval,
use_grid=False,
)

if self.mode in ["fast", "fast with events"] or not model.events:
if not model.events:
pybamm.logger.info("No events found, running fast mode")
Expand Down
232 changes: 0 additions & 232 deletions pybamm/solvers/processed_symbolic_variable.py

This file was deleted.

Loading

0 comments on commit cd640a5

Please sign in to comment.