-
-
Notifications
You must be signed in to change notification settings - Fork 738
Added mypy enable_error_code sp check guideline
#4891
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
de39912
2d7a327
81f74bf
59ac4f3
a3895c7
1ae758b
b04f766
dfdd7ff
160f279
0a42b57
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,12 +34,12 @@ def process_2D(name, data): | |
| D_s_n_data = process_2D("Negative particle diffusivity [m2.s-1]", df) | ||
|
|
||
|
|
||
| def D_s_n(sto, T): | ||
| def D_s_n_func(sto, T): | ||
|
||
| name, (x, y) = D_s_n_data | ||
| return pybamm.Interpolant(x, y, [T, sto], name) | ||
|
|
||
|
|
||
| parameter_values["Negative particle diffusivity [m2.s-1]"] = D_s_n | ||
| parameter_values["Negative particle diffusivity [m2.s-1]"] = D_s_n_func | ||
|
|
||
| k_n = parameter_values["Negative electrode exchange-current density [A.m-2]"] | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -140,7 +140,7 @@ def __init__( | |
| self.value = pybamm.Interpolant( | ||
| t, | ||
| y, | ||
| pybamm.t - pybamm.InputParameter("start time"), | ||
| pybamm.t - pybamm.InputParameter("start time"), # type: ignore[arg-type] | ||
|
||
| name="Drive Cycle", | ||
| ) | ||
| self.period = np.diff(t).min() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,6 @@ | |
| # Binary operator classes | ||
| # | ||
| from __future__ import annotations | ||
| import numbers | ||
|
|
||
| import numpy as np | ||
| import numpy.typing as npt | ||
|
|
@@ -34,8 +33,8 @@ def _preprocess_binary( | |
| raise ValueError("right must be a 1D array") | ||
| right = pybamm.Vector(right) | ||
|
|
||
| # Check both left and right are pybamm Symbols | ||
| if not (isinstance(left, pybamm.Symbol) and isinstance(right, pybamm.Symbol)): | ||
| # Check right is pybamm Symbol | ||
| if not isinstance(right, pybamm.Symbol): | ||
| raise NotImplementedError( | ||
| f"BinaryOperator not implemented for symbols of type {type(left)} and {type(right)}" | ||
| ) | ||
|
|
@@ -114,6 +113,9 @@ def __str__(self): | |
| right_str = f"{self.right!s}" | ||
| return f"{left_str} {self.name} {right_str}" | ||
|
|
||
| def _new_instance(self, left: pybamm.Symbol, right: pybamm.Symbol) -> pybamm.Symbol: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please add some information on why this was added?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've replaced error: Missing positional argument "right_child" in call to "BinaryOperator" [call-arg]but this function was always getting called from instance of its child classes which don't need to pass 3 arguments, so i thought it was better to make a new_instance method which can be overrided in child classes I've already added this in the PR description of previous sp-check-guidelines PR, should I add it here too? Or follow some different approach |
||
| return self.__class__(self.name, left, right) # pragma: no cover | ||
|
|
||
| def create_copy( | ||
| self, | ||
| new_children: list[pybamm.Symbol] | None = None, | ||
|
|
@@ -128,7 +130,7 @@ def create_copy( | |
| children = self._children_for_copying(new_children) | ||
|
|
||
| if not perform_simplifications: | ||
| out = self.__class__(children[0], children[1]) | ||
| out = self._new_instance(children[0], children[1]) | ||
| else: | ||
| # creates a new instance using the overloaded binary operator to perform | ||
| # additional simplifications, rather than just calling the constructor | ||
|
|
@@ -225,6 +227,9 @@ def __init__( | |
| """See :meth:`pybamm.BinaryOperator.__init__()`.""" | ||
| super().__init__("**", left, right) | ||
|
|
||
| def _new_instance(self, left: pybamm.Symbol, right: pybamm.Symbol) -> pybamm.Symbol: | ||
| return Power(left, right) | ||
|
|
||
| def _diff(self, variable: pybamm.Symbol): | ||
| """See :meth:`pybamm.Symbol._diff()`.""" | ||
| # apply chain rule and power rule | ||
|
|
@@ -274,6 +279,9 @@ def __init__( | |
| """See :meth:`pybamm.BinaryOperator.__init__()`.""" | ||
| super().__init__("+", left, right) | ||
|
|
||
| def _new_instance(self, left: pybamm.Symbol, right: pybamm.Symbol) -> pybamm.Symbol: | ||
| return Addition(left, right) | ||
|
|
||
| def _diff(self, variable: pybamm.Symbol): | ||
| """See :meth:`pybamm.Symbol._diff()`.""" | ||
| return self.left.diff(variable) + self.right.diff(variable) | ||
|
|
@@ -301,6 +309,9 @@ def __init__( | |
|
|
||
| super().__init__("-", left, right) | ||
|
|
||
| def _new_instance(self, left: pybamm.Symbol, right: pybamm.Symbol) -> pybamm.Symbol: | ||
| return Subtraction(left, right) | ||
|
|
||
| def _diff(self, variable: pybamm.Symbol): | ||
| """See :meth:`pybamm.Symbol._diff()`.""" | ||
| return self.left.diff(variable) - self.right.diff(variable) | ||
|
|
@@ -330,6 +341,9 @@ def __init__( | |
|
|
||
| super().__init__("*", left, right) | ||
|
|
||
| def _new_instance(self, left: pybamm.Symbol, right: pybamm.Symbol) -> pybamm.Symbol: | ||
| return Multiplication(left, right) | ||
|
|
||
| def _diff(self, variable: pybamm.Symbol): | ||
| """See :meth:`pybamm.Symbol._diff()`.""" | ||
| # apply product rule | ||
|
|
@@ -370,6 +384,9 @@ def __init__( | |
| """See :meth:`pybamm.BinaryOperator.__init__()`.""" | ||
| super().__init__("@", left, right) | ||
|
|
||
| def _new_instance(self, left: pybamm.Symbol, right: pybamm.Symbol) -> pybamm.Symbol: | ||
| return MatrixMultiplication(left, right) # pragma: no cover | ||
|
|
||
| def diff(self, variable): | ||
| """See :meth:`pybamm.Symbol.diff()`.""" | ||
| # We shouldn't need this | ||
|
|
@@ -419,6 +436,9 @@ def __init__( | |
| """See :meth:`pybamm.BinaryOperator.__init__()`.""" | ||
| super().__init__("/", left, right) | ||
|
|
||
| def _new_instance(self, left: pybamm.Symbol, right: pybamm.Symbol) -> pybamm.Symbol: | ||
| return Division(left, right) | ||
|
|
||
| def _diff(self, variable: pybamm.Symbol): | ||
| """See :meth:`pybamm.Symbol._diff()`.""" | ||
| # apply quotient rule | ||
|
|
@@ -467,6 +487,9 @@ def __init__( | |
| """See :meth:`pybamm.BinaryOperator.__init__()`.""" | ||
| super().__init__("inner product", left, right) | ||
|
|
||
| def _new_instance(self, left: pybamm.Symbol, right: pybamm.Symbol) -> pybamm.Symbol: | ||
| return Inner(left, right) # pragma: no cover | ||
|
|
||
| def _diff(self, variable: pybamm.Symbol): | ||
| """See :meth:`pybamm.Symbol._diff()`.""" | ||
| # apply product rule | ||
|
|
@@ -544,6 +567,9 @@ def __init__( | |
| """See :meth:`pybamm.BinaryOperator.__init__()`.""" | ||
| super().__init__("==", left, right) | ||
|
|
||
| def _new_instance(self, left: pybamm.Symbol, right: pybamm.Symbol) -> pybamm.Symbol: | ||
| return Equality(left, right) | ||
|
|
||
| def diff(self, variable): | ||
| """See :meth:`pybamm.Symbol.diff()`.""" | ||
| # Equality should always be multiplied by something else so hopefully don't | ||
|
|
@@ -601,6 +627,10 @@ def __init__( | |
| ): | ||
| """See :meth:`pybamm.BinaryOperator.__init__()`.""" | ||
| super().__init__(name, left, right) | ||
| self.name = name | ||
|
|
||
| def _new_instance(self, left: pybamm.Symbol, right: pybamm.Symbol) -> pybamm.Symbol: | ||
| return _Heaviside(self.name, left, right) # pragma: no cover | ||
|
|
||
| def diff(self, variable): | ||
| """See :meth:`pybamm.Symbol.diff()`.""" | ||
|
|
@@ -679,6 +709,9 @@ def __init__( | |
| ): | ||
| super().__init__("%", left, right) | ||
|
|
||
| def _new_instance(self, left: pybamm.Symbol, right: pybamm.Symbol) -> pybamm.Symbol: | ||
| return Modulo(left, right) | ||
|
|
||
| def _diff(self, variable: pybamm.Symbol): | ||
| """See :meth:`pybamm.Symbol._diff()`.""" | ||
| # apply chain rule and power rule | ||
|
|
@@ -721,6 +754,9 @@ def __init__( | |
| ): | ||
| super().__init__("minimum", left, right) | ||
|
|
||
| def _new_instance(self, left: pybamm.Symbol, right: pybamm.Symbol) -> pybamm.Symbol: | ||
| return Minimum(left, right) | ||
|
|
||
| def __str__(self): | ||
| """See :meth:`pybamm.Symbol.__str__()`.""" | ||
| return f"minimum({self.left!s}, {self.right!s})" | ||
|
|
@@ -765,6 +801,9 @@ def __init__( | |
| ): | ||
| super().__init__("maximum", left, right) | ||
|
|
||
| def _new_instance(self, left: pybamm.Symbol, right: pybamm.Symbol) -> pybamm.Symbol: | ||
| return Maximum(left, right) | ||
|
|
||
| def __str__(self): | ||
| """See :meth:`pybamm.Symbol.__str__()`.""" | ||
| return f"maximum({self.left!s}, {self.right!s})" | ||
|
|
@@ -1539,7 +1578,7 @@ def source( | |
| corresponding to a source term in the bulk. | ||
| """ | ||
| # Broadcast if left is number | ||
| if isinstance(left, numbers.Number): | ||
| if isinstance(left, (int, float)): | ||
| left = pybamm.PrimaryBroadcast(left, "current collector") | ||
|
|
||
| # force type cast for mypy | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,8 +78,7 @@ def _from_json(cls, snippet): | |
| ) | ||
|
|
||
| def _unary_new_copy(self, child: pybamm.Symbol, perform_simplifications=True): | ||
| """See :meth:`pybamm.UnaryOperator._unary_new_copy()`.""" | ||
| return self.__class__(child, self.broadcast_domain) | ||
| pass # pragma: no cover | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this removed?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In |
||
|
|
||
|
|
||
| class PrimaryBroadcast(Broadcast): | ||
|
|
@@ -191,6 +190,10 @@ def reduce_one_dimension(self): | |
| """Reduce the broadcast by one dimension.""" | ||
| return self.orphans[0] | ||
|
|
||
| def _unary_new_copy(self, child: pybamm.Symbol, perform_simplifications=True): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are these functions being repeated?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As mentioned above now it needs to be overrided in child classes |
||
| """See :meth:`pybamm.UnaryOperator._unary_new_copy()`.""" | ||
| return self.__class__(child, self.broadcast_domain) | ||
|
|
||
|
|
||
| class PrimaryBroadcastToEdges(PrimaryBroadcast): | ||
| """A primary broadcast onto the edges of the domain.""" | ||
|
|
@@ -321,6 +324,10 @@ def reduce_one_dimension(self): | |
| """Reduce the broadcast by one dimension.""" | ||
| return self.orphans[0] | ||
|
|
||
| def _unary_new_copy(self, child: pybamm.Symbol, perform_simplifications=True): | ||
| """See :meth:`pybamm.UnaryOperator._unary_new_copy()`.""" | ||
| return self.__class__(child, self.broadcast_domain) | ||
|
|
||
|
|
||
| class SecondaryBroadcastToEdges(SecondaryBroadcast): | ||
| """A secondary broadcast onto the edges of a domain.""" | ||
|
|
@@ -438,6 +445,10 @@ def reduce_one_dimension(self): | |
| """Reduce the broadcast by one dimension.""" | ||
| raise NotImplementedError | ||
|
|
||
| def _unary_new_copy(self, child: pybamm.Symbol, perform_simplifications=True): | ||
| """See :meth:`pybamm.UnaryOperator._unary_new_copy()`.""" | ||
| return self.__class__(child, self.broadcast_domain) | ||
|
|
||
|
|
||
| class TertiaryBroadcastToEdges(TertiaryBroadcast): | ||
| """A tertiary broadcast onto the edges of a domain.""" | ||
|
|
@@ -463,7 +474,7 @@ def __init__( | |
| self, | ||
| child_input: Numeric | pybamm.Symbol, | ||
| broadcast_domain: DomainType = None, | ||
| auxiliary_domains: AuxiliaryDomainType = None, | ||
| auxiliary_domains: AuxiliaryDomainType | str = None, | ||
| broadcast_domains: DomainsType = None, | ||
| name: str | None = None, | ||
| ): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,6 +77,8 @@ def __init__(self, name="Unnamed model"): | |
| self.use_jacobian = True | ||
| self.convert_to_format = "casadi" | ||
|
|
||
| self.calculate_sensitivities = [] | ||
|
||
|
|
||
| # Model is not initially discretised | ||
| self.is_discretised = False | ||
| self.y_slices = None | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,8 +94,8 @@ def supports_parallel_solve(self): | |
| def requires_explicit_sensitivities(self): | ||
| return True | ||
|
|
||
| @root_method.setter | ||
| def root_method(self, method): | ||
| @root_method.setter # type: ignore[attr-defined, no-redef] | ||
| def root_method(self, method) -> None: | ||
| if method == "casadi": | ||
| method = pybamm.CasadiAlgebraicSolver(self.root_tol) | ||
| elif isinstance(method, str): | ||
|
|
@@ -1122,7 +1122,7 @@ def _set_sens_initial_conditions_from( | |
| """ | ||
|
|
||
| ninputs = len(model.calculate_sensitivities) | ||
| initial_conditions = tuple([] for _ in range(ninputs)) | ||
| initial_conditions: tuple = tuple([] for _ in range(ninputs)) | ||
|
||
| solution = solution.last_state | ||
| for var in model.initial_conditions: | ||
| final_state = solution[var.name] | ||
|
|
@@ -1143,10 +1143,10 @@ def _set_sens_initial_conditions_from( | |
| slices = [y_slices[symbol][0] for symbol in model.initial_conditions.keys()] | ||
|
|
||
| # sort equations according to slices | ||
| concatenated_initial_conditions = [ | ||
| concatenated_initial_conditions = tuple( | ||
| casadi.vertcat(*[eq for _, eq in sorted(zip(slices, init))]) | ||
| for init in initial_conditions | ||
| ] | ||
| ) | ||
| return concatenated_initial_conditions | ||
|
|
||
| def process_t_interp(self, t_interp): | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.