Deprecate number of electrodes in parallel parameter

[vidipsingh]

Description

This PR deprecates the n_electrodes_parallel parameter due to its inconsistent usage across the codebase. Area calculations in the thermal models have been updated to use the total effective area instead.

Fixes: #4833

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #)

Important checks:

Please confirm the following before marking the PR as ready for review:

• No style issues: nox -s pre-commit
• All tests pass: nox -s tests
• The documentation builds: nox -s doctests
• Code is commented for hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[vidipsingh]

Need to decide what to do about Ai2020 and Sulzer2019 - perhaps just scaling the electrode width. Also this will need a clear deprecation message (we could check specifically for when people give a value that is not equal to 1 and raise a warning in that case)

@valentinsulzer Thanks for the suggestion! For Ai2020 and Sulzer2019, I'll look into scaling the electrode width as you mentioned.

Regarding the deprecation message, I can add a check for when the n_electrodes_parallel value is not equal to 1 and raise a warning to inform users about the change. Could you please guide me on where exactly these messages should be added in the code?

[DrSOKane]

I see why you want to do this but it can be a bit of a shock to people new to PyBaMM. I was just mentoring someone who was struggling to get the correct capacity and didn't realize that they needed to double the electrode length (compared to what it was in the paper) to account for it being double-sided, which implies the authors of that paper did it a different way than Chen et al.

[aabills]

Thanks for taking this on, @vidipsingh . I can't help but notice that there are a bunch of changes made here that are unrelated to this issue. Perhaps they are related to #4802 ? It would be good to start this from a clean branch to ensure that nothing extraneous is considered here.

[aabills]

I see why you want to do this but it can be a bit of a shock to people new to PyBaMM. I was just mentoring someone who was struggling to get the correct capacity and didn't realize that they needed to double the electrode length (compared to what it was in the paper) to account for it being double-sided, which implies the authors of that paper did it a different way than Chen et al.

My argument is just that we should either make everything consistent with this parameter, or deprecate it, of which I thought deprecation made more sense. Maybe we should debate this at the original issue, #4833

[vidipsingh]

Thanks for taking this on, @vidipsingh . I can't help but notice that there are a bunch of changes made here that are unrelated to this issue. Perhaps they are related to #4802 ? It would be good to start this from a clean branch to ensure that nothing extraneous is considered here.

Thanks for pointing that out, @aabills. I did start from a clean branch, so I'm not entirely sure how the changes from that PR ended up here. What would you suggest I do at this point? If I start from a new branch, I'll likely need to close this PR and raise a new one.

[aabills]

Thanks for taking this on, @vidipsingh . I can't help but notice that there are a bunch of changes made here that are unrelated to this issue. Perhaps they are related to #4802 ? It would be good to start this from a clean branch to ensure that nothing extraneous is considered here.

Thanks for pointing that out, @aabills. I did start from a clean branch, so I'm not entirely sure how the changes from that PR ended up here. What would you suggest I do at this point? If I start from a new branch, I'll likely need to close this PR and raise a new one.

Seems reasonable to me, make sure you check out discussion in #4833 first.

[vidipsingh]

Thanks for taking this on, @vidipsingh . I can't help but notice that there are a bunch of changes made here that are unrelated to this issue. Perhaps they are related to #4802 ? It would be good to start this from a clean branch to ensure that nothing extraneous is considered here.

Thanks for pointing that out, @aabills. I did start from a clean branch, so I'm not entirely sure how the changes from that PR ended up here. What would you suggest I do at this point? If I start from a new branch, I'll likely need to close this PR and raise a new one.

Seems reasonable to me, make sure you check out discussion in #4833 first.

Sure, will check that.

[rtimms]

Fixed by https://github.com/pybamm-team/PyBaMM/pull/5138