Fix BatteryElectrochemEODEOL model

[lymichelle21]

Address #199 with [Option 3] by reworking the combined BatteryElectroChemEODEOL model so that the relevant parameters do not change inside the model and instead stay as states. This prevents the overriding behavior with qMobile, tDiffusion, and Ro in the original implementation. The parameters affected by side effects are now handled locally in their respective functions.

This way, we can still use LazySim so event_state can be calculated only as needed without intermediate printing. This option is less memory intensive than [Option 1] and less computationally expensive than [Option 2]. The main downside is some instances of duplicated code. There is some refactoring introduced to help mitigate this.

[github-actions]

Thank you for opening this PR. Each PR into dev requires a code review. For the code review, look at the following:

• Reviewer (someone other than author) should look for bugs, efficiency, readability, testing, and coverage in examples (if relevant).
• Ensure that each PR adding a new feature should include a test verifying that feature.
• All errors from static analysis must be resolved.
• Review the test coverage reports (if there is a change) - will be added as comment on PR if there is a change
• Review the software benchmarking results (if there is a change) - will be added as comment on PR
• Any added dependencies are included in requirements.txt, setup.py, and dev_guide.rst (this document)
• All warnings from static analysis must be reviewed and resolved - if deemed appropriate.

[teubert]

Great job with a very complex task.

I think there one more wrinkle with qMax to unravel, then we should be done

[teubert]

I don't think we need qMax here anymore. Now it is calculated from qMobile as we go since qMobile is changing.

This means your helper function will have to receive qMax as a parameter now.

There are 2 derived x0 from qMax that I don't think you need anymore either. since it's dependent on another state those will then need to be calculated when you call init

We can talk more about this tomorrow

[kjjarvis]

Just one suggestion while we wait to finalize the qMax changes

[kjjarvis]

Great comment, very clear!

[kjjarvis]

Maybe we can try to make this just a tad more specific. I'm wondering if, months from now when we've forgotten this issue, we'll be confused with what we mean by "original model". Similarly, I wonder if we'll understand the hardcoded values below. I like that you link to the PR, this is great, but maybe we could also make the text just a bit more descriptive? This is just an idea, edit as desired:

"Test that the current combined model has the same results as an initial implementation. Note that this initial implementation is no longer in ProgPy. While we have confidence in the quantitative results, a development challenged forced the need for a separate EODEOL model. In the below test, some values from this initial implementation are hard-coded for testing purposes. (See link for more detail)."