Introducing energy demand for warm water heating #1358
Conversation
# Conflicts: # src/main/scala/edu/ie3/simona/model/thermal/ThermalHouse.scala # src/test/scala/edu/ie3/simona/agent/grid/ThermalGridIT.scala # src/test/scala/edu/ie3/simona/model/participant/HpModelSpec.scala # src/test/scala/edu/ie3/simona/model/thermal/ThermalGridWithHouseAndStorageSpec.scala # src/test/scala/edu/ie3/simona/model/thermal/ThermalGridWithHouseOnlySpec.scala # src/test/scala/edu/ie3/simona/model/thermal/ThermalHouseSpec.scala
# Conflicts: # src/test/scala/edu/ie3/simona/agent/grid/ThermalGridIT.scala # src/test/scala/edu/ie3/simona/model/thermal/ThermalGridWithHouseAndStorageSpec.scala # src/test/scala/edu/ie3/simona/model/thermal/ThermalGridWithHouseOnlySpec.scala # src/test/scala/edu/ie3/simona/model/thermal/ThermalHouseSpec.scala
# Conflicts: # src/test/scala/edu/ie3/simona/agent/grid/ThermalGridIT.scala
There was a problem hiding this comment.
Pull Request Overview
This PR introduces energy demand for warm water heating in SIMONA's thermal modeling system. It adds domestic hot water storage functionality to thermal grids and enhances the thermal house model to handle both space heating and hot water demands.
Key changes include:
- Addition of domestic hot water storage components and their integration into thermal grids
- Enhanced thermal grid logic to handle multiple storage types and demand scenarios
- Refactored feed-in strategy patterns for distributing thermal power across grid elements
Reviewed Changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ThermalGrid.scala | Major refactoring to support domestic hot water storage, new demand calculation methods, and feed-in strategy patterns |
| ThermalHouse.scala | Added import ordering fix for better code organization |
| ThermalThreshold.scala | Added SimpleThermalThreshold case class for generic threshold handling |
| ThermalStrategyPatterns.scala | New file defining feed-in distribution strategies |
| ThermalStorageType.scala | New type definitions for storage type safety |
| ThermalResult.scala | New result handling classes for different thermal components |
| ThermalDemandConditions.scala | New conditions evaluation logic for thermal demand scenarios |
| HpModel.scala | Updated to handle domestic hot water demand determination and enhanced operating point structure |
| Test files | Extensive updates to test data and specifications to accommodate new domestic hot water functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
# Conflicts: # CHANGELOG.md # src/main/scala/edu/ie3/simona/model/thermal/ThermalDemandConditions.scala # src/main/scala/edu/ie3/simona/model/thermal/ThermalGrid.scala
# Conflicts: # CHANGELOG.md
# Conflicts: # src/main/scala/edu/ie3/simona/model/thermal/ThermalGrid.scala # src/main/scala/edu/ie3/simona/model/thermal/ThermalStrategyPatterns.scala # src/test/scala/edu/ie3/simona/agent/grid/ThermalGridIT.scala # src/test/scala/edu/ie3/simona/model/thermal/ThermalGridWithHouseAndStorageSpec.scala
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
staudtMarius
left a comment
staudtMarius
left a comment
There was a problem hiding this comment.
Here are some comments from my side. I will have a closer look at the tests later.
src/main/scala/edu/ie3/simona/model/participant/hp/HpModel.scala
Outdated
Show resolved
Hide resolved
| demandHeatStorage.hasRequiredDemand || | ||
| (demandHeatStorage.hasPossibleDemand && wasRunningLastPeriod)) | ||
| (demandHeatStorage.hasPossibleDemand && wasRunningLastPeriod)) || | ||
| demandDomesticHotWaterStorage.hasRequiredDemand |
There was a problem hiding this comment.
DId you forget the check for demandDomesticHotWaterStorage.hasPossibleDemand && wasRunningLastPeriod?
Also, why is this check no inside the previous bracket, while the normal heat storage is?
There was a problem hiding this comment.
What do you mean be the check for demandDomesticHotWaterStorage.hasPossibleDemand && wasRunningLastPeriod?
But one of the brackets was wrong. Good catch!
| demandHouse.hasRequiredDemand || demandHouse.hasPossibleDemand || | ||
| demandHeatStorage.hasRequiredDemand || demandHeatStorage.hasPossibleDemand | ||
| demandHeatStorage.hasRequiredDemand || demandHeatStorage.hasPossibleDemand || | ||
| demandDomesticHotWaterStorage.hasRequiredDemand |
There was a problem hiding this comment.
Why are you only checking for required demand here, but no the possible demand?
There was a problem hiding this comment.
The idea was and is to keep the domestic hot water storage out of the flexibility patterns. This can be discussed but I would like to keep it simple as it is still complicated enough... I added some phrase to the docs.
src/main/scala/edu/ie3/simona/model/participant/hp/HpModel.scala
Outdated
Show resolved
Hide resolved
| * If the house was heated in lastState and has still some demand and the domestic | ||
| * hot water storage has no demand. */ |
There was a problem hiding this comment.
Why are we stopping the heating if the hot water storage has some demand?
There was a problem hiding this comment.
We don't stop. This only checks if we can continue heating the house as in last time step. Therefore we check:
- was house heated last step
- does it still has some demand
- has the hot water storage NO demand (if it would have some, we need to change the heat distribution and share between house and water storage)
resolves #856
merge first
ThermalGrid.determineMostRecentThreshold()#1478