Add blkt type enum#4212
Conversation
There was a problem hiding this comment.
Pull request overview
Introduces an IntEnum to represent blanket model types (i_blanket_type) and replaces hard-coded blanket type integers with enum members in several core/model locations.
Changes:
- Added
BlktModelTypes(IntEnum)(currently covering CCFE HCPB and DCLL) to centralize blanket type identifiers. - Updated blanket type checks in core execution/output paths and in the power model to use the enum.
- Updated DCLL-specific geometry branching in the blanket library to use the enum.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| process/models/power.py | Uses BlktModelTypes instead of magic blanket-type integers in thermal efficiency logic. |
| process/models/blankets/blanket_library.py | Adds BlktModelTypes enum and updates DCLL type branching to use it. |
| process/core/output.py | Uses BlktModelTypes for selecting which blanket model’s output to write. |
| process/core/init.py | Uses BlktModelTypes in validation logic for CCFE HCPB blanket settings. |
| process/core/caller.py | Uses BlktModelTypes for selecting which blanket model to run. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from process.core.solver import constraints | ||
| from process.core.solver.iteration_variables import set_scaled_iteration_variable | ||
| from process.core.solver.objectives import objective_function | ||
| from process.models.blankets.blanket_library import BlktModelTypes |
There was a problem hiding this comment.
Importing BlktModelTypes from process.models.blankets.blanket_library couples the core caller to the full blanket library import graph. Consider relocating the enum to a lightweight module (or importing locally) so selecting a blanket model doesn't require importing blanket_library.py at module import time.
| if fwbs_variables.i_thermal_electric_conversion == 0: | ||
| # CCFE HCPB Model | ||
| if fwbs_variables.i_blanket_type == 1: | ||
| if fwbs_variables.i_blanket_type == BlktModelTypes.CCFE_HCPB: | ||
| # HCPB, efficiency taken from M. Kovari 2016 | ||
| # "PROCESS": A systems code for fusion power plants - Part 2: Engineering | ||
| # https://www.sciencedirect.com/science/article/pii/S0920379616300072 |
There was a problem hiding this comment.
After switching to BlktModelTypes.CCFE_HCPB here, the corresponding else branch immediately below still calls logger.log(...) with only a message and text that references blanket type 1. logger.log requires a level argument and will raise TypeError if that branch is hit; update it to logger.warning/info(...) (or logger.log(logging.<LEVEL>, ...)) and make the message consistent with the enum.
| elif fwbs_variables.i_thermal_electric_conversion == 1: | ||
| # CCFE HCPB Model | ||
| if fwbs_variables.i_blanket_type == 1: | ||
| if fwbs_variables.i_blanket_type == BlktModelTypes.CCFE_HCPB: | ||
| # HCPB, efficiency taken from M. Kovari 2016 | ||
| # "PROCESS": A systems code for fusion power plants - Part 2: Engineering | ||
| # https://www.sciencedirect.com/science/article/pii/S0920379616300072 |
There was a problem hiding this comment.
The else branch for this blanket-type check (just below this block) uses logger.log(...) without providing a log level and references blanket type 1 in the message. This will raise at runtime if non-CCFE-HCPB blanket types use i_thermal_electric_conversion == 1; switch to logger.warning/info(...) (or logger.log(logging.<LEVEL>, ...)) and update the message to reference BlktModelTypes.CCFE_HCPB consistently.
| from process.models.blankets.blanket_library import BlktModelTypes | ||
|
|
There was a problem hiding this comment.
Importing BlktModelTypes from process.models.blankets.blanket_library pulls in blanket_library's heavy dependencies (e.g., CoolProp via process.core.coolprop_interface) just to access an enum. Consider moving BlktModelTypes into a lightweight module (e.g., process.data_structure.fwbs_variables or a small process.models.blankets.types module) and importing from there to reduce coupling and import-time side effects.
| from process.models.blankets.blanket_library import BlktModelTypes | |
| def _get_blkt_model_types(): | |
| from process.models.blankets.blanket_library import BlktModelTypes | |
| return BlktModelTypes | |
| class _BlktModelTypesProxy: | |
| def __getattr__(self, name): | |
| return getattr(_get_blkt_model_types(), name) | |
| BlktModelTypes = _BlktModelTypesProxy() |
| class BlktModelTypes(IntEnum): | ||
| """Enum for blanket model types. `i_blanket_type`""" | ||
|
|
||
| CCFE_HCPB = 1 | ||
| DCLL = 5 |
There was a problem hiding this comment.
Defining BlktModelTypes inside blanket_library.py makes any import of the enum also import the full blanket library (numpy + CoolProp interface, etc.). To avoid unnecessary heavyweight imports in core modules, consider relocating this enum to a lightweight module (e.g., process.data_structure.fwbs_variables where i_blanket_type is defined) and importing it here instead.
| @@ -1,5 +1,6 @@ | |||
| from process import data_structure | |||
| from process.core.log import logging_model_handler | |||
| from process.models.blankets.blanket_library import BlktModelTypes | |||
There was a problem hiding this comment.
Importing BlktModelTypes from process.models.blankets.blanket_library introduces an eager dependency on the full blanket library (and its heavy imports) even for cases that return early (stellarator/IFE). Consider importing the enum from a lightweight module (or doing a local import inside write) to avoid unnecessary import-time side effects.
| ) | ||
| from process.data_structure.tfcoil_variables import init_tfcoil_variables | ||
| from process.data_structure.times_variables import init_times_variables | ||
| from process.models.blankets.blanket_library import BlktModelTypes |
There was a problem hiding this comment.
Importing BlktModelTypes from process.models.blankets.blanket_library pulls in the full blanket library (including CoolProp interface) during core initialization. Consider moving the enum to a lightweight module (e.g., process.data_structure.fwbs_variables) to reduce import-time coupling and potential side effects.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4212 +/- ##
==========================================
+ Coverage 52.10% 52.12% +0.01%
==========================================
Files 148 148
Lines 30389 30397 +8
==========================================
+ Hits 15835 15843 +8
Misses 14554 14554 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
je-cook
left a comment
There was a problem hiding this comment.
this needs the pre validation with the enum so we cant pass unknown values through the system
i_blanket_type = BlktModelTypes(i_blanket_type)
Co-authored-by: Copilot <copilot@github.com>
…readability and maintainability Co-authored-by: Copilot <copilot@github.com>
10f6b05 to
33d1a37
Compare
…cy in power calculations Co-authored-by: Copilot <copilot@github.com>
Description
Checklist
I confirm that I have completed the following checks: