-
Notifications
You must be signed in to change notification settings - Fork 4
Feature/cooling switch temperature based #100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Harm <harm.van.leijen@alliander.com>
…or temperature dependant changes in the time constants Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
… in transformers Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
…dates during calculations Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
d62633d to
2b9f742
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new feature you added is quite complex at first sight.
The readability of the code could be improved with some more documentation in the code by more elaborate docstrings or online comments.
- Updated pre-commit hooks to latest versions. - Modified example notebook to include ONAN parameters and adjust execution counts. - Refactored test cases to utilize ONAN parameters for ONAF switch. - Introduced CoolingSwitchController to encapsulate ONAN/ONAF switching logic. - Enhanced Model class to validate transformer and input profile compatibility. - Updated transformer base class to support cooling controller. - Added ONAN and ThreeWindingONAN parameters for detailed cooling specifications. - Refactored ONAFSwitch to include ONAN parameters and ensure proper validation. - Adjusted PowerTransformer and ThreeWindingTransformer to utilize the new cooling controller. - Implemented logic to handle cooling type switching based on fan status and temperature thresholds. Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
…ulations Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
Signed-off-by: Harm <harm.van.leijen@alliander.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces dynamic cooling mode switching between ONAN (Oil Natural Air Natural) and ONAF (Oil Natural Air Forced) for transformer thermal calculations, allowing users to model transformers with fan-controlled cooling.
Key changes:
- Introduced
CoolingSwitchControllerto manage cooling mode transitions based on fan schedules or temperature thresholds - Refactored thermal calculations from vectorized to loop-based processing to support dynamic parameter changes
- Added schema classes
ONAFSwitch,ONANParameters, and related three-winding variants for configuration
Reviewed Changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| transformer_thermal_model/transformer/cooling_switch_controller.py | New controller class managing ONAN/ONAF switching logic and specification updates |
| transformer_thermal_model/schemas/thermal_model/onaf_switch.py | New schema classes for switch configuration and ONAN parameters |
| transformer_thermal_model/transformer/base.py | Added cooling controller support and delegation methods for switching |
| transformer_thermal_model/transformer/power.py | Integrated cooling controller and updated temperature calculation signature |
| transformer_thermal_model/transformer/threewinding.py | Integrated cooling controller for three-winding transformers |
| transformer_thermal_model/transformer/distribution.py | Updated to pass None for cooling controller |
| transformer_thermal_model/model/thermal_model.py | Refactored to loop-based calculations with per-timestep specification updates |
| transformer_thermal_model/schemas/specifications/transformer.py | Fixed array construction to remove unnecessary nested brackets |
| tests/model/test_onan_onaf_switch.py | Comprehensive test suite for switching functionality |
| docs/get_started/about.md | Added extensive documentation with examples |
| mkdocs.yml | Added documentation references for new feature |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| ) | ||
|
|
||
| def check_switch_and_get_new_specs( | ||
| self, top_oil_temp: int, previous_top_oil_temp: int, index: int |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The top_oil_temp and previous_top_oil_temp parameters are typed as int, but they should be float since they represent temperature values that can have decimal precision.
| self, top_oil_temp: int, previous_top_oil_temp: int, index: int | |
| self, top_oil_temp: float, previous_top_oil_temp: float, index: int |
| class BaseONANParameters(BaseModel): | ||
| """Base class representing common ONAN (Oil Natural Air Natural) cooling parameters. | ||
| THis is used when an ONAF transformer switches to ONAN cooling. |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected spelling of 'THis' to 'This'.
| THis is used when an ONAF transformer switches to ONAN cooling. | |
| This is used when an ONAF transformer switches to ONAN cooling. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already looking very good!
My major suggestion is to change the naming of the onaf_switch schemas to something like cooling_switch/ CoolingSwitch.
Furthermore, you have introduced two functions with the same name. One in the transformer and one in the coolingswitch controller. please reconsider this.
| Note, how the top oil temperature we receive as the output `results.top_oil_temp_profile` exactly matches | ||
| the top oil temperature we provided as the input. | ||
|
|
||
| ### Use a transformer that switches between ONAN and ONAF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ### Use a transformer that switches between ONAN and ONAF | |
| ### Model a transformer that switches between ONAN and ONAF |
| """Check if the configuration is valid. | ||
| If the transformer is a ThreeWindingTransformer, the input profile must be a ThreeWindingInputProfile. | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ik zou de docstring iets simpeler houden. je hoeft niet de if statements in de docstring te benoemen.
| """Check if the configuration is valid. | |
| If the transformer is a ThreeWindingTransformer, the input profile must be a ThreeWindingInputProfile. | |
| """ | |
| """Check if the combination of the transformer and input profile the is valid. | |
| """ |
| initial_top_oil_temperature=init_top_oil_temp | ||
| ) | ||
|
|
||
| def check_switch_and_get_new_specs( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not follow the "single purpose" rule according to the function name. The name of the function gives this away: def do_this_AND_do_that()..
But looking into the function it seems to be quite simple.
maybe change the name to something like: " set_cooling_switch_controller_specs"?
| from transformer_thermal_model.schemas.specifications.transformer import WindingSpecifications | ||
|
|
||
|
|
||
| class FanSwitchConfig(BaseModel): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make these schemas correspondent better with functions and classes in the cooling_switch_controller.py please consider to use coolingcontroller instead of onafswitch in the naming.
| class FanSwitchConfig(BaseModel): | |
| class CoolingSwitchConfig(BaseModel): |
| load_loss_hv_mv: float | ||
|
|
||
|
|
||
| class ONAFSwitchBase(BaseModel): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| class ONAFSwitchBase(BaseModel): | |
| class CoolingSwitchBase(BaseModel): |
| return self | ||
|
|
||
|
|
||
| class ONAFSwitch(ONAFSwitchBase): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| class ONAFSwitch(ONAFSwitchBase): | |
| class CoolingSwitchSettings(CoolingSwitchBase): |
| ) | ||
|
|
||
|
|
||
| class ThreeWindingONAFSwitch(ONAFSwitchBase): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This name will become very long... so please consider if you have any other creative naming options
| class ThreeWindingONAFSwitch(ONAFSwitchBase): | |
| class ThreeWindingCoolingSwitchSettings(CoolingSwitchBase): |
|
|
||
| def __init__( | ||
| self, | ||
| onaf_switch: ONAFSwitch | ThreeWindingONAFSwitch, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This attributes name could be changed to cooling_switch
| onaf_switch: ONAFSwitch | ThreeWindingONAFSwitch, | |
| cooling_switch: CoolingSwitch | ThreeWindingCoolingSwitch, |
|
|
||
| return transformer_specs | ||
|
|
||
| def check_switch_and_get_new_specs( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a function in the base transformer with exact the same name. please reconsider if this is intentionally and otherwise change the naming.
| def check_switch_and_get_new_specs( |
This PR introduces the ability to configure the switch between ONAN (Oil Natural Air Natural) and ONAF (Oil Natural Air Forced) cooling modes for transformer calculations. This enhancement allows users to:
Created a
CoolingSwitchControllerclass that encapsulates all logic associated with switching between ONAN and ONAF states. The controller preserves original ONAF specifications and dynamically alters the specs with the user provided ONAN parameters when the fans turn off. It supports both two-winding and three-winding transformers.Calculation Method Changes:
New Schema Components:
ONAFSwitchandThreeWindingONAFSwitch: Configuration classes for switch settings.ONANParametersandThreeWindingONANParameters: Defines the transformer specifications to use when in ONAN mode.FanSwitchConfig: Contains activation and deactivation temperature thresholds for temperature-based switchingPerformance Trade-offs
The move from vectorized calculations to loop-based processing increases computation time. However, this trade-off is justified because current use cases rarely require high-performance computation. Additionally, in future versions we may want to add time dependant parameters, that would also require, this new method of calculating.
Example Usage
To model with the switch between ONAN and ONAF, create a Transformer with the additional parameter
cooling_switch_settings: