refactor: split Simulation into BaseSimulation and Simulation #5430
Merged
BradyPlanden merged 8 commits intomainfrom Apr 1, 2026
Merged
refactor: split Simulation into BaseSimulation and Simulation #5430BradyPlanden merged 8 commits intomainfrom
BradyPlanden merged 8 commits intomainfrom
Conversation
Extract a BaseSimulation class for non-experiment simulations and refactor Simulation to inherit from it, adding experiment support. This improves separation of concerns and maintainability. - BaseSimulation: core build/solve/step, eSOH, plotting, serialisation - Simulation(BaseSimulation): experiment orchestration, state mapping, unified/legacy model modes, cycle/step loop - Extract _prepare_solve() and _discretise_experiment_models() helpers to eliminate code duplication between base and experiment paths - Add _recompute_initial_conditions override in Simulation to clear per-step solver caches (correctness fix) - Replace magic strings with class-level constants (MODE_WITHOUT_EXPERIMENT, MODE_DRIVE_CYCLE, MODE_WITH_EXPERIMENT, _PADDING_REST_KEY) - Consolidate __setstate__ backward-compat guards into _SETSTATE_DEFAULTS - Update isinstance checks in plotting modules to use BaseSimulation - Add test_base_simulation.py with 23 tests covering class hierarchy, solve/build, pickle, properties, and plotting acceptance
Performance: - Replace termination staticmethods with class-level string constants (_STEP_INDEX_INPUT, _TERMINATION_TIME/VOLTAGE/CAPACITY, _TERMINATION_FINAL_TIME, _TERMINATION_EXPERIMENT_TAG, _COMBINED_TERMINATION_EVENT) to avoid function call overhead - Hoist loop-invariant attribute lookups (experiment_steps, uses_unified, step_indices) as locals before the experiment solve loop - Move Serialise import to local scope in save_model to avoid loading serialisation machinery at module import time - Add early return in _build_experiment_state_mappers for unified model path Idiom improvements: - Extract _should_save_cycle() and _check_infeasible_steps() helpers to reduce solve() complexity - Simplify dt computation, initial_start_time ternary, and voltage_stop conditional - Deprecate _experiment_step_index_input_name() in favour of _STEP_INDEX_INPUT constant - Update base_solver.py to use class constant directly - Remove WHAT-not-WHY comments throughout
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5430 +/- ##
==========================================
+ Coverage 98.31% 98.34% +0.03%
==========================================
Files 331 333 +2
Lines 30471 30475 +4
==========================================
+ Hits 29957 29971 +14
+ Misses 514 504 -10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Cover previously untested methods: _pv_fingerprint, _normalize_inputs, _compute_esoh_fingerprint, set_initial_state (cache_esoh=False path), _prepare_solve (calc_esoh warning), _get_built_models, plot/create_gif/ plot_voltage_components error paths, deprecated method warnings, save_model edge cases, _should_save_cycle, _check_infeasible_steps, experiment model mode validation, unified vs legacy input builders, state mapper edge cases, and experiment type coercion.
MarcBerliner
previously approved these changes
Mar 31, 2026
Member
MarcBerliner
left a comment
MarcBerliner
left a comment
There was a problem hiding this comment.
Thanks @BradyPlanden, this is a nice cleanup
MarcBerliner
approved these changes
Apr 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extract a BaseSimulation class for non-experiment simulations and refactor Simulation to inherit from it, adding experiment support.
unified/legacy model modes, cycle/step loop
to eliminate code duplication between base and experiment paths
per-step solver caches (correctness fix)
(MODE_WITHOUT_EXPERIMENT, MODE_DRIVE_CYCLE, MODE_WITH_EXPERIMENT,
_PADDING_REST_KEY)
solve/build, pickle, properties, and plotting acceptance# Description
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:
nox -s pre-commitnox -s testsnox -s doctests