Update existing solvers to new interface #1177
Merged
Conversation
This file contains 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
* Refactor test helper into new module * Use ParameterFrame not Dict in Task constructor * Start on Plasmod interface refactor - Setup * Add plasmod Run task The run_subprocess function has been moved to a separate utility. This allows us to mock our own `run_subprocess` function in tests, and not worry about if we want to change the implementation of how we run our shell commands. E.g., if we wanted to start using os.system rather than subprocess.Popen, for whatever reason. * Refactor plasmod solver interface's Teardown * Fix bug in run_subprocess + add test reminder * Fix param mappings in refactored Plasmod interface * Fix dataclass attribute/method ordering Apparently dataclass attributes need to be declared first for the corresponding kwarg to be added to the constructor. Declare the methods after the attributes to fix this. * Add missing Plasmod Teardown tests * Remove extraneous dict init in plasmod.params * Add refactored plasmod Solver class * Add end-to-end test for plasmod solver * Add 'cwol' to PlasmodOutput dataclass * Add test for error on bad Plasmod result flag * Add test for writing PlasmodInput params * Convert PlasmodInput models to enums on init * Create new PlasmodInputs on Solver.update_inputs This allows for the post-init processing to do its magic. * Fix typing in plasmod MODEL_MAP * Replace old plasmod Solver with refactored * Add warning if plasmod output param has no value Adds back change made in #1009 after refactor. * Split up large files in plasmod.solver * Allow using enum name to specify plasmod models * Warn when using unrecognised inputs in plasmod * Add get_profile back into plasmod.Solver This was temporarily removed as part of a refactor. * Add get_profiles method back into plasmod.Solver This had been temporarily removed during a refactor. * Add 'scalar_outputs' method to plasmod.Solver This replaces the previous 'get_raw_variables' function. Using a structure as an output means that users know exactly what attributes are allowed. IDEs can then make suggestions about attributes, and also spot typos that wouldn't be spotted using strings. * Allow param edits between calls of plasmod solver * Put modify_mappings back into plasmod solver * Flesh out plasmod Task constructor docstrings * Update plasmod example to use new solver interface * Add CodesSolver base class This is intended to replace the FileProgramInterface class, as part of the refactor of the solvers interface. * Move plasmod.Solver functionality to base class * Extract plasmod.Solver general setup to base class * Re-add 'jiter' as plasmod output parameter This had been lost in a refactor. * Separate profiles and scalars in plasmod outputs * Move plasmod exit code check to Teardown task This makes more sense than throwing when reading the outputs. * Undo change in plasmod example's transport model This was an accidental change made during a refactor. * Change to a better method in CodesTeardown * Use plasmod.Profiles enums in Sovler.get_profile This is in-line with the effort towards: "All text based choices should be enumified" #1044 * Remove resolved TODO * Write tests to resolve TODO * Resolve flake8 warnings * Resolve ambiguity in plasmod solver Setup var name This attempts to resolve the ambiguity around the 'input_file' variable name. * Update plasmod solver directory structure This is more consistent with the structure in PROCESS's solver * Fix plasmod example's get_profile usage * Fix sphinx build * Fix file path in flake8 ignores I'd forgotten to update the file path after moving the file. * Fix import in tests * Simplify import in plasmod example * Add missing type hint in CodesTask._run_subprocess * Add Optional to typing * Add better typing + docstrings to udpate_inputs
* Process api additions (#1055) * ✨ process species additions * 🎨 improvements spurred by comments * ✅ add a test * 🐛 pass if process not installed * 🐛 pass if process not installed * Update bluemira/codes/process/api.py Co-authored-by: Harry Saunders <33317174+hsaunders1904@users.noreply.github.com> Co-authored-by: Harry Saunders <33317174+hsaunders1904@users.noreply.github.com> * Fix PROCESS enum value typo CYRO -> CRYO * Add refactored Setup class for PROCESS solver * Add refactored Run class for PROCESS solver * Remove unused attribute in test class * Clean up logging in plasmod Run class This logging is more consistent with what's in PROCESS, solver and more specific about what's happening. * Write tests ahead of PROCESS Teardown refactor * Make PROCESS solver's Teardown tests pass * Add refactored PROCESS solver class * Remove old PROCESS solver implementation * Fix typo in test that was causing it to pass * Correct docstring in PROCESS solver * Remove trailing commas from PROCESS test IN.DAT Parameters whose values contained a trailing comma could not be read by PROCESS. The file format is no longer supported by PROCESS, so we no longer support it either. Remove the trailing commas so the IN.DAT file is usable in tests. * Fix PROCESS test not stopping mock.patch This was causing the patched function being called in subsequent tests! Whoops * Add tests for PROCESS run modes * Fix some find-replace errors * Make solver's Teardown testable without PROCESS Mock out the PROCESS file reader so that the Teardown class's logic can be testing without PROCESS being installed. * Catch case where PROCESS result missing param PROCESS does not necessarily output all the values we need for a radial build. There are certainly some MFiles which are missing some parameters. Catch these cases and raise a better error. * Resolve PROCESS solver TODO The below was not needed. The files are not required to run the solver. * Fix unused variable in PROCESS solver * Remove check for empty file This will cause an error later on anyway, so the check is not needed. * Fix misbehaving PROCESS tests One of the tests was passing, but because an error of a similar type was caught. Check the error message to make sure the right error is thrown. * Add missing docstrings to PROCESS Teardown * Remove unused constant in test * Replace old codes.interface module * Restore 'plot_radial_build' to PROCESS API * Add run_mode_cls property to Solver This allows us to access the available run modes for a solver, without prior knowledge of which Solver class it is. * Update codes.wrapper to use new Solver interface * Add __all__ imports to codes.plasmod * Use run and read directories in PROCESS Teardown * Fix file paths in PROCESS system tests * Update codes.interface tests * Add some missing docstrings in PROCESS solver * Fix flake8 warning in base.design * Fix test that fails for long git branch names The look_and_feel.print_banner test fails if the git branch is too long. The long name caused a new line in the banner, which made the asserted banner length incorrect. * Restore species methods to PROCESS solver These had been removed as part of the refactor of the PROCESS API. * Fix mocking of PROCESS module The mock was overwriting the PROCESS module, instead of patching it. The effect of this was causing a subsequent test to fail. * Remove superfluous comment * Use walrus operator to summarise if statement * Remove unused imports Co-authored-by: je-cook <81617086+je-cook@users.noreply.github.com>
* Update EUDEMO power cycle solver for new interface * Remove erroneously restored file after rebase A bad rebase restored a previously deleted file. Re-delete it
* Update Solver API example Updates the example to use a ParameterFrame. * Update PROCESS solver example * Tidy solvers with walrus operator
|
@ivanmaione This is the PR that contains the Plasmod changes I mentioned. #1054 might give a (slightly) more succinct overview of the changes. Key points are:
Let me know if there's anything that needs clarifying. |
je-cook
approved these changes
Jun 22, 2022
|
Kudos, SonarCloud Quality Gate passed!
|
Codecov Report
@@ Coverage Diff @@
## develop #1177 +/- ##
===========================================
+ Coverage 74.19% 75.32% +1.13%
===========================================
Files 179 189 +10
Lines 21147 21486 +339
===========================================
+ Hits 15690 16185 +495
+ Misses 5457 5301 -156
Continue to review full report at Codecov.
|
je-cook
deleted the
hsaunders1904/956_move_existing_solvers_to_new_interface
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Linked Issues
Closes #956
Description
Migrates the Plasmod and PROCESS solvers to the new solver interface. This is a feature branch being merged, so is a combination of several other PRs:
Interface Changes
PROCESS and Plasmod solvers.
Checklist
I confirm that I have completed the following checks:
pytest tests --reactorflake8andblack .sphinx-build -W documentation/source documentation/build