PyBaMM: Raise error if updating a parameter that already exists with `check_already_exists=False`

This will force people to use check_already_exists=False sparingly, and help to catch bugs related to misnaming parameters (this is caught if check_already_exists=True, but not if it’s False

About this issue

  • Original URL
  • State: open
  • Created 2 years ago
  • Comments: 18 (13 by maintainers)

Most upvoted comments

I added a portion of this change in #3382. Although that PR only logs a warning. I did a partial change due to the number of tests and examples that would need to be updated by the change.

For making it an error to update parameters with check_already_exists=False, I think one of the following two options could be good:

  1. Change check_already_exists to something along the lines of add_new_parameters. My reasoning is that check_already_exists=True sounds like “make sure you don’t overwrite parameters” while check_already_exists=False sounds like “Don’t check, just create or update”. It would be less confusing if the variable name was closer to the intended usage once the error is in place.
  2. Split the update() function into two different functions. There could be an update_parameter() and an add_new_parameter(). This would get rid of check_already_exists and avoid confusion.

Of those two options, I would lean towards (2) since each function would have a single purpose and there would be no ambiguity from the name of the variable.