Change "Cell capacity [Ah]" to "Nominal cell capacity [Ah]"

[valentinsulzer]

Hopefully this will make it clear that this parameter is only for converting C-rate to current.

[Saransh-cpp]

Hi there I am new here, does this issue only needs renaming?

[brosaplanella]

Hi @Saransh-cpp! Yes. You should also add a deprecation warning, but we can help you with that if you don't know where to place it.

[Saransh-cpp]

Yes thanks! That would be great. I have been exploring the codebase and looking for the "Cell capacity [Ah]" parameter but had no luck finding it. Can you please guide me by providing a file or a directory path where this is used?

[brosaplanella]

Sorry about that, there is a typo. You have to look for "Cell capacity [A.h]". Check if now you can find it :)

[Saransh-cpp]

@brosaplanella thanks for the correction. I found a lot of references in the .csv of different cell under the input directory, in the test directory and also in files like simulation.py, electrical_parameters.py, parameter_values.py, test_lead_acid_parameters.py etc. Should I rename all the references to "Nominal cell capacity [A.h]"?

[brosaplanella]

That's correct. Running the tests locally will help you spot if you missed any.

[Saransh-cpp]

Okay! Sorry I got a bit busy, I'll start working on it and try the deprecated warning also, if I have any problems I'll post them here:).

[Saransh-cpp]

@brosaplanella I should not rename Cell capacity in the CHANGELOG.md file right? Should I add a new line there saying 'Cell capacity [A.h]' has been deprecated. The Nominal cell capacity now can be accessed as 'Nominal cell capacity [A.h]'. and how do I proceed with the deprecated warning? Thanks

[brosaplanella]

Exactly, you shouldn't rename it in CHANGELOG.md as it lists an old change. The CHANGELOG is a list of the changes in the code so you shouldn't place the deprecation warning there. Once you are done with your changes, you should add a new line to CHANGELOG explaining what you change and link it to the corresponding pull request (PR), so you will have to open a PR first before editing CHANGELOG so you know what is the PR number.

For the deprecating warning, it is a bit more tricky. We want to throw a warning saying that it will be deprecated in the next release but make the code still work with the current notation. You can find an example of a deprecation warning in line164 of parameter_values.py. In that case, we changed the notation from anode to negative electrode so what we do is pass the anode chemistry into the negative electrode chemistry and then raise a warning so the user can fix it before the next release (where it will break).

[valentinsulzer]

The changelog does also list breaking changes and deprecations (though I don't know whether anyone actually looks at those)

[brosaplanella]

Sorry, my bad, I was thinking of the actual deprecation warning. An example on CHANGELOG is on line 78

• The parameters "Positive/Negative particle distribution in x" and "Positive/Negative surface area to volume ratio distribution in x" have been deprecated. Instead, users can provide "Positive/Negative particle radius [m]" and "Positive/Negative surface area to volume ratio [m-1]" directly as functions of through-cell position (x [m]) (#1237)

[Saransh-cpp]

Okay! I'll get on it. Thanks

[Saransh-cpp]

I've been trying but I don't completely understand where I should add the deprecated warning? Should I create another if condition after line 343 or 354 in parameter_values.py because I think that is the function where the values are being checked?

[brosaplanella]

Yes, that's it. What you can do is add a new block like the one on "C-rate" (l. 338-343) where you check if "Cell capacity [A.h]" is in there. If so, check if "Nominal cell capacity [A.h]" is in there too, and if both are present raise an error. If only "Cell capacity [A.h]" is in there, define values["Nominal cell capacity [A.h]"] = values["Cell capacity [A.h]"] and then raise a warning.

For the deprecation warning see the example in parameter_values.py line 165.