Conversation
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
|
@FJanssen-TNO Ready for review |
| else 10.0e6 | ||
| ) | ||
|
|
||
| # The asset attribute "dischargeEfficiency" represents the fraction of stored heat |
There was a problem hiding this comment.
The two name conventions conflict
- efficiency vs fraction
- dischargeEfficiency vs stored heat lost
I would expect that if I put in 98%, that only 2% is lost during the day.
There was a problem hiding this comment.
"heat_loss_efficiency " is renamed as "heat_loss_coefficient". With this naming, we are aligned with the "efficiency attribute" definitions of esdl. Besides, using heat_loss_coefficienct looks more logical now
src/mesido/esdl/esdl_heat_model.py
Outdated
| # that is lost per day. If this attribute is not provided in esdl, a default heat loss rate | ||
| # of 1% per day is used. The value is converted to a per‑second loss factor. |
There was a problem hiding this comment.
The comment "if this attribute is not provided ...." can be removed, this can be seen below.
There was a problem hiding this comment.
rest of the comment is removed
src/mesido/esdl/esdl_heat_model.py
Outdated
| # The asset attribute "dischargeEfficiency" represents the fraction of stored heat | ||
| # that is lost per day. If this attribute is not provided in esdl, a default heat loss rate | ||
| # of 1% per day is used. The value is converted to a per‑second loss factor. | ||
| heat_loss_efficiency = ( |
There was a problem hiding this comment.
Name convention is confusing: heat loss efficiency is confliction, so either efficiency or heat loss coefficient. The latter is actually what you are using.
There was a problem hiding this comment.
renamed as "heat_loss_coefficient"
| self.height = 5.0 | ||
| self.radius = 10.0 | ||
| self.volume = math.pi * self.radius**2 * self.height | ||
| self.heat_loss_coeff = 2 * self.heat_transfer_coeff / (self.radius * self.rho * self.cp) |
There was a problem hiding this comment.
maybe keep this old line as commented code as in the future we might want to use a heat transfer coefficient together with its dimensions.
There was a problem hiding this comment.
old definition of "heat_loss_coefficient" is added back again as a comment
| # Test that if heat_loss_efficiency is incorporated into heat loss calculation of buffer | ||
| esdl_asset = solution.esdl_assets[solution.esdl_asset_name_to_id_map["HeatStorage_74c1"]] | ||
| np.testing.assert_allclose( | ||
| parameters["HeatStorage_74c1.heat_loss_efficiency"], | ||
| esdl_asset.attributes["dischargeEfficiency"] / (24.0 * 3600.0), | ||
| ) | ||
| np.testing.assert_allclose( | ||
| results["HeatStorage_74c1.Heat_loss"], | ||
| results["HeatStorage_74c1.Stored_heat"] | ||
| * parameters["HeatStorage_74c1.heat_loss_efficiency"], | ||
| ) | ||
|
|
There was a problem hiding this comment.
This test is about sizing. The checks that you have done here, should be for instance in the other test where you are checking the buffer.
There was a problem hiding this comment.
checks are removed from "test_max_size_and_optional_assets.py" and placed into "test_warmingup_unit_cases.py". Anyways, one of the check was already done in test_warmingup_unit_cases. I have only added check related to to "dischargeEfficiency" attribute
| # that is lost per day. If this attribute is not provided in esdl, a default heat loss rate | ||
| # of 1% per day is used. The value is converted to a per‑second loss factor. | ||
| heat_loss_efficiency = ( | ||
| asset.attributes.get("dischargeEfficiency") / (24.0 * 3600.0) |
There was a problem hiding this comment.
| asset.attributes.get("dischargeEfficiency") / (24.0 * 3600.0) | |
| asset.attributes.get("dischargeEfficiency") /100 / (24.0 * 3600.0) |
There was a problem hiding this comment.
Don't do this suggestion, appearantly efficiencies are now always fractions in esdl. Lets atleast in our own code be clear and name it fraction/coefficient etc.
There was a problem hiding this comment.
I renamed "heat_loss_efficiency " as "heat_loss_coefficient". Hence, this aligns with the definition of efficiency attributes used in esdl
CHANGELOG.md
Outdated
| ## Added | ||
| - Parsing of ensemble profiles when using input profiles from csv. | ||
| - Maximum profile constraint for PV asset is considered in PV sizing | ||
| - heat_loss_efficiency of HeatStorage is read from dischargeEfficiency attribute in esdl |
There was a problem hiding this comment.
| - heat_loss_efficiency of HeatStorage is read from dischargeEfficiency attribute in esdl | |
| - DischargeEfficiency is parsed from HeatStorage assets in ESDL to a heat loss coefficient |
There was a problem hiding this comment.
suggestion is implemented
# Conflicts: # CHANGELOG.md
|
@FJanssen-TNO Ready for review |
Done: