Skip to content
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

Renamed 'Cell capacity [A.h]' to 'Nominal cell capacity [A.h]' #1352

1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
- `Interpolant` now takes `x` and `y` instead of a single `data` entry ([#1312](https://github.com/pybamm-team/PyBaMM/pull/1312))
- Boolean model options ('sei porosity change', 'convection') must now be given in string format ('true' or 'false' instead of True or False) ([#1280](https://github.com/pybamm-team/PyBaMM/pull/1280))
- Operations such as `1*x` and `0+x` now directly return `x`. This can be bypassed by explicitly creating the binary operators, e.g. `pybamm.Multiplication(1, x)` ([#1252](https://github.com/pybamm-team/PyBaMM/pull/1252))
- `'Cell capacity [A.h]'` has been renamed to `'Nominal cell capacity [A.h]'`. `'Cell capacity [A.h]'` will be deprecated in the next release. ([#1352](https://github.com/pybamm-team/PyBaMM/pull/1352))

# [v0.3.0](https://github.com/pybamm-team/PyBaMM) - 2020-11-22

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
"source": [
"chemistry = pybamm.parameter_sets.Ai2020\n",
"param = pybamm.ParameterValues(chemistry=chemistry)\n",
"capacity = param[\"Cell capacity [A.h]\"]\n",
"capacity = param[\"Nominal cell capacity [A.h]\"]\n",
"param.update({\n",
" \"Current function [A]\": capacity * pybamm.InputParameter(\"C-rate\")\n",
"})\n",
Expand Down Expand Up @@ -334,4 +334,4 @@
},
"nbformat": 4,
"nbformat_minor": 4
}
}
4 changes: 2 additions & 2 deletions examples/notebooks/compare-ecker-data.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@
"outputs": [],
"source": [
"C_rates = [1, 5] # C-rates to solve for\n",
"capacity = parameter_values[\"Cell capacity [A.h]\"]\n",
"capacity = parameter_values[\"Nominal cell capacity [A.h]\"]\n",
"t_evals = [\n",
" np.linspace(0, 3800, 100), \n",
" np.linspace(0, 720, 100)\n",
Expand Down Expand Up @@ -266,4 +266,4 @@
},
"nbformat": 4,
"nbformat_minor": 2
}
}
4 changes: 2 additions & 2 deletions examples/notebooks/models/pouch-cell-model.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@
"outputs": [],
"source": [
"param = dfn.default_parameter_values\n",
"I_1C = param[\"Cell capacity [A.h]\"] # 1C current is cell capacity multipled by 1 hour\n",
"I_1C = param[\"Nominal cell capacity [A.h]\"] # 1C current is cell capacity multipled by 1 hour\n",
"param.update(\n",
" {\n",
" \"Current function [A]\": I_1C * 3, \n",
Expand Down Expand Up @@ -854,4 +854,4 @@
},
"nbformat": 4,
"nbformat_minor": 2
}
}
4 changes: 2 additions & 2 deletions examples/notebooks/models/thermal-models.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@
],
"source": [
"# update current to correspond to a C-rate of 3 (i.e. 3 times the nominal cell capacity)\n",
"parameter_values[\"Current function [A]\"] = 3 * parameter_values[\"Cell capacity [A.h]\"]\n",
"parameter_values[\"Current function [A]\"] = 3 * parameter_values[\"Nominal cell capacity [A.h]\"]\n",
"\n",
"# pick solver \n",
"solver = pybamm.CasadiSolver(mode=\"fast\", atol=1e-3)\n",
Expand Down Expand Up @@ -504,4 +504,4 @@
},
"nbformat": 4,
"nbformat_minor": 4
}
}
4 changes: 2 additions & 2 deletions examples/notebooks/speed-up-solver.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@
"# Set up model\n",
"model = pybamm.lithium_ion.DFN()\n",
"param = model.default_parameter_values\n",
"cap = param[\"Cell capacity [A.h]\"]\n",
"cap = param[\"Nominal cell capacity [A.h]\"]\n",
"param[\"Current function [A]\"] = cap * pybamm.InputParameter(\"Crate\")\n",
"sim = pybamm.Simulation(model, parameter_values=param)\n",
"\n",
Expand Down Expand Up @@ -5666,4 +5666,4 @@
},
"nbformat": 4,
"nbformat_minor": 4
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Cell cooling surface area [m2],1.54E-1,,
Cell volume [m3],2.70E-4,,
,,,
# Electrical,,,
Cell capacity [A.h],17,Manufacturer,
Nominal cell capacity [A.h],17,Manufacturer,
Typical current [A],1,,
Current function [A],1,default current function,
,,,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ Negative current collector thermal conductivity [W.m-1.K-1],401,CRC Handbook,cop
Positive current collector thermal conductivity [W.m-1.K-1],237,CRC Handbook,aluminium
,,,
# Electrical,,,
Cell capacity [A.h],1.1,Lain 2019,
Nominal cell capacity [A.h],1.1,Lain 2019,
Current function [A],4.4,,
Typical current [A],4.4,Lain 2019,
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ Negative current collector thermal conductivity [W.m-1.K-1],401,CRC Handbook,cop
Positive current collector thermal conductivity [W.m-1.K-1],237,CRC Handbook,aluminium
,,,
# Electrical,,,
Cell capacity [A.h], 2.28,Ai 2020,2.28/34
Nominal cell capacity [A.h], 2.28,Ai 2020,2.28/34
Typical current [A], 2.28,Ai 2020,
Current function [A], 2.28,default current function,
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@ Negative current collector thermal conductivity [W.m-1.K-1],267.467, 401 * 0.667
Positive current collector thermal conductivity [W.m-1.K-1],158.079, 237 * 0.667,
,,,
# Electrical,,,
Cell capacity [A.h],0.43,trial and error,
Nominal cell capacity [A.h],0.43,trial and error,
Typical current [A],0.43,0.2857,1C current
Current function [A],0.43,default current function,
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ Negative current collector thermal conductivity [W.m-1.K-1],401,CRC Handbook,cop
Positive current collector thermal conductivity [W.m-1.K-1],237,CRC Handbook,aluminium
,,,
# Electrical,,,
Cell capacity [A.h],5,Chen 2020,
Nominal cell capacity [A.h],5,Chen 2020,
Typical current [A],5,Chen 2020,
Current function [A],5,default current function,
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ Negative current collector thermal conductivity [W.m-1.K-1],401,,
Positive current collector thermal conductivity [W.m-1.K-1],237,,
,,,
# Electrical,,,
Cell capacity [A.h],5,Peyman MPM,
Nominal cell capacity [A.h],5,Peyman MPM,
Typical current [A],5,,1C current
Current function [A],5,default current function,
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ Cell cooling surface area [m2],1.72E-2,,pouch (single layer)
Cell volume [m3],1.61E-6,,pouch (single layer)
,,,
# Electrical,,,
Cell capacity [A.h], 0.15625, 7.5/48 (parameter set for a single layer cell),
Nominal cell capacity [A.h], 0.15625, 7.5/48 (parameter set for a single layer cell),
Typical current [A], 0.15652,,
Current function [A],0.15652,default current function,
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@ Negative current collector thermal conductivity [W.m-1.K-1],401,,
Positive current collector thermal conductivity [W.m-1.K-1],237,,
,,,
# Electrical,,,
Cell capacity [A.h],0.680616,,24 Ah/m2 * 0.137m * 0.207m
Nominal cell capacity [A.h],0.680616,,24 Ah/m2 * 0.137m * 0.207m
Typical current [A],0.680616,,1C current
Current function [A],0.680616,default current function,
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ Negative current collector thermal conductivity [W.m-1.K-1],401,,
Positive current collector thermal conductivity [W.m-1.K-1],237,,
,,,
# Electrical,,,
Cell capacity [A.h],1,,16.54 Ah/m2 * 0.057m * 1.06m
Nominal cell capacity [A.h],1,,16.54 Ah/m2 * 0.057m * 1.06m
Typical current [A],1,,1C current
Current function [A],1,default current function,
2 changes: 1 addition & 1 deletion pybamm/parameters/electrical_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def _set_dimensional_parameters(self):
"Defines the dimensional parameters"

self.I_typ = pybamm.Parameter("Typical current [A]")
self.Q = pybamm.Parameter("Cell capacity [A.h]")
self.Q = pybamm.Parameter("Nominal cell capacity [A.h]")
self.C_rate = pybamm.AbsoluteValue(self.I_typ / self.Q)
self.n_electrodes_parallel = pybamm.Parameter(
"Number of electrodes connected in parallel to make a cell"
Expand Down
26 changes: 23 additions & 3 deletions pybamm/parameters/parameter_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,9 +338,29 @@ def check_parameter_values(self, values):
if "C-rate" in values:
raise ValueError(
"The 'C-rate' parameter has been deprecated, "
"use 'Current function [A]' instead. The cell capacity can be accessed "
"as 'Cell capacity [A.h]', and used to calculate current from C-rate."
"use 'Current function [A]' instead. The Nominal "
"cell capacity can be accessed as 'Nominal cell "
"capacity [A.h]', and used to calculate current from C-rate."
)
if "Cell capacity [A.h]" in values:
if "Nominal cell capacity [A.h]" in values:
raise ValueError(
"both 'Cell capacity [A.h]' and 'Nominal cell capacity [A.h]' "
"provided in values. The 'Cell capacity [A.h]' notation will be "
"deprecated in the next release so 'Nominal cell capacity [A.h]' "
"should be used instead."
)
else:
values["Nominal cell capacity [A.h]"] = values["Cell capacity [A.h]"]
warnings.warn(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to pass DeprecationWarning as an argument (see below) otherwise it raises a generic warning and that's why the test fails.

                warnings.warn(
                    "the 'Cell capacity [A.h]' notation will be "
                    "deprecated in the next release, as it has now been renamed "
                    "to 'Nominal cell capacity [A.h]'. Simulation will continue "
                    "passing the 'Cell capacity [A.h]' as 'Nominal cell "
                    "capacity [A.h]' (it might overwrite any existing definition "
                    "of the component)",
                    DeprecationWarning,
                )

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, my bad, I didn't run the tests on my side. Running them now for confirmation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After adding this and running the tests, it gives me an error

pybamm.ParameterValues({"Cell capacity [A.h]": 0})
AssertionError: "Cell capacity [A.h]" does not match "the 'Cell capacity [A.h]' notation will be deprecated in the next release, as it has now been renamed to 'Nominal cell capacity [A.h]'. Simulation 
will continue passing the 'Cell capacity [A.h]' as 'Nominal cell capacity [A.h]' (it might overwrite any existing definition of the component)"

How can I resolve this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know... Maybe try passing only "Cell capacity". Also, I don't think it is the source of any issue, but might be good to define the capacities in the test to be 1 rather than 0, as a 0 capacity could potentially lead to further issues.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that but then it shows the "Deprecation warning not triggered" error. I am looking into it, if you find the issue please let me know. Thanks

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I did that, it does not give me the "Deprecation warning not triggered" with "Cell capacity [A.h]" but it does give me the same error with "Cell capacity".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you push your recent changes so I can see the full error messages from the automatic tests?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes sure, I was trying to use values=, but that didn't work out as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it locally and it seems the issue is with [A.h]. It seems that this runs fine:

        # Cell capacity [A.h] deprecated
        with self.assertRaisesRegex(ValueError, "Cell capacity"):
            pybamm.ParameterValues({"Cell capacity [A.h]": 1,
                                    "Nominal cell capacity [A.h]": 1})
        with self.assertWarnsRegex(DeprecationWarning, "Cell capacity"):
            pybamm.ParameterValues({"Cell capacity [A.h]": 1})

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much!

"the 'Cell capacity [A.h]' notation will be "
"deprecated in the next release, as it has now been renamed "
"to 'Nominal cell capacity [A.h]'. Simulation will continue "
"passing the 'Cell capacity [A.h]' as 'Nominal cell "
"capacity [A.h]' (it might overwrite any existing definition "
"of the component)",
DeprecationWarning,
)
for param in values:
if "surface area density" in param:
raise ValueError(
Expand Down Expand Up @@ -791,7 +811,7 @@ def print_parameters(self, parameters, output_file=None):
# Calculate parameters for each C-rate
for Crate in [1, 10]:
# Update Crate
capacity = self.get("Cell capacity [A.h]")
capacity = self.get("Nominal cell capacity [A.h]")
if capacity is not None:
self.update(
{"Current function [A]": Crate * capacity},
Expand Down
6 changes: 3 additions & 3 deletions pybamm/simulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def __init__(
self._parameter_values.update(
{
"Current function [A]": self.C_rate
* self._parameter_values["Cell capacity [A.h]"]
* self._parameter_values["Nominal cell capacity [A.h]"]
}
)

Expand Down Expand Up @@ -218,7 +218,7 @@ def set_up_experiment(self, model, experiment):
I = op[0]
else:
# Scale C-rate with capacity to obtain current
capacity = self._parameter_values["Cell capacity [A.h]"]
capacity = self._parameter_values["Nominal cell capacity [A.h]"]
I = op[0] * capacity
operating_inputs = {
"Current switch": 1,
Expand Down Expand Up @@ -264,7 +264,7 @@ def set_up_experiment(self, model, experiment):
I = events[0]
else:
# Scale C-rate with capacity to obtain current
capacity = self._parameter_values["Cell capacity [A.h]"]
capacity = self._parameter_values["Nominal cell capacity [A.h]"]
I = events[0] * capacity
operating_inputs.update(
{"Current cut-off [A]": I, "Voltage cut-off [V]": -1e10}
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/test_experiments.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def test_discharge_rest_charge(self):
np.testing.assert_array_almost_equal(
sim._solution["Time [h]"].entries, np.linspace(0, 3, 13)
)
cap = model.default_parameter_values["Cell capacity [A.h]"]
cap = model.default_parameter_values["Nominal cell capacity [A.h]"]
np.testing.assert_array_almost_equal(
sim._solution["Current [A]"].entries,
[cap / 2] * 5 + [0] * 4 + [-cap / 2] * 4,
Expand Down Expand Up @@ -62,7 +62,7 @@ def test_gitt(self):
np.testing.assert_array_almost_equal(
sim._solution["Time [h]"].entries, np.arange(0, 20.01, 0.1)
)
cap = model.default_parameter_values["Cell capacity [A.h]"]
cap = model.default_parameter_values["Nominal cell capacity [A.h]"]
np.testing.assert_array_almost_equal(
sim._solution["Current [A]"].entries,
[cap / 20] * 11 + [0] * 10 + ([cap / 20] * 10 + [0] * 10) * 9,
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/test_models/standard_model_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def test_solving(self, solver=None, t_eval=None):

Crate = abs(
self.parameter_values["Current function [A]"]
* self.parameter_values["Cell capacity [A.h]"]
* self.parameter_values["Nominal cell capacity [A.h]"]
)
# don't allow zero C-rate
if Crate == 0:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def test_set_up(self):
self.assertEqual(sim.experiment, experiment)
self.assertEqual(
sim._experiment_inputs[0]["Current input [A]"],
1 / 20 * model.default_parameter_values["Cell capacity [A.h]"],
1 / 20 * model.default_parameter_values["Nominal cell capacity [A.h]"],
)
self.assertEqual(sim._experiment_inputs[0]["Current switch"], 1)
self.assertEqual(sim._experiment_inputs[0]["Voltage switch"], 0)
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/test_parameters/test_lead_acid_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def test_all_defined(self):
output_file = "lead_acid_parameters.txt"
parameter_values.print_parameters(parameters, output_file)
# test print_parameters with dict and without C-rate
del parameter_values["Cell capacity [A.h]"]
del parameter_values["Nominal cell capacity [A.h]"]
parameters = {"C_e": parameters.C_e, "sigma_n": parameters.sigma_n}
parameter_values.print_parameters(parameters)

Expand Down
6 changes: 6 additions & 0 deletions tests/unit/test_parameters/test_parameter_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,12 @@ def test_update(self):
param.update({"b": 1})

def test_check_parameter_values(self):
# Cell capacity [A.h] deprecated
with self.assertRaisesRegex(ValueError, "Cell capacity"):
pybamm.ParameterValues({"Cell capacity [A.h]": 1,
"Nominal cell capacity [A.h]": 1})
with self.assertWarnsRegex(DeprecationWarning, "Cell capacity"):
pybamm.ParameterValues({"Cell capacity [A.h]": 1})
# Can't provide a current density of 0, as this will cause a ZeroDivision error
with self.assertRaisesRegex(ValueError, "Typical current"):
pybamm.ParameterValues({"Typical current [A]": 0})
Expand Down