modelica-buildings
modelica-buildings copied to clipboard
Issue2912 district cooling
@khinkelman Please see the inline comments. I have also committed some changes aada46d, 9bd530d, 4a87098, 551c8a3, to clean up models and reference results.
The model Buildings.Experimental.DHC.Examples.Cooling.DirectUncontrolledETS
failed to run with Optimica and OpenModelica with following errors:
See 'simulator-optimica.log' for details.
409*** Error: Simulation of Buildings.Experimental.DHC.Examples.Cooling.DirectUncontrolledETS failed: 'FMUException: Failed to update the events at time: 1.572482E+07.'. Directory is '/tmp/tmp-Buildings-0-xlzir_zy'.
410Execution time = 1452.984 s
411make: *** [Makefile:150: test-optimica] Error 1
See 'simulator-openmodelica.log' for details.
385*** Error: Simulation of Buildings.Experimental.DHC.Examples.Cooling.DirectUncontrolledETS failed: 'CalledProcessError: Command '['omc', 'Buildings.Experimental.DHC.Examples.Cooling.DirectUncontrolledETS_simulate.mos']' returned non-zero exit status 1.'. Directory is '/tmp/tmp-Buildings-0-exrbuqkj'.
386Execution time = 1724.656 s
387make: *** [Makefile:145: test-openmodelica] Error 1
@khinkelman Please see the inline comments. I have also committed some changes aada46d, 9bd530d, 4a87098, 551c8a3, to clean up models and reference results. The model
Buildings.Experimental.DHC.Examples.Cooling.DirectUncontrolledETS
failed to run with Optimica and OpenModelica with following errors:See 'simulator-optimica.log' for details. 409*** Error: Simulation of Buildings.Experimental.DHC.Examples.Cooling.DirectUncontrolledETS failed: 'FMUException: Failed to update the events at time: 1.572482E+07.'. Directory is '/tmp/tmp-Buildings-0-xlzir_zy'. 410Execution time = 1452.984 s 411make: *** [Makefile:150: test-optimica] Error 1
See 'simulator-openmodelica.log' for details. 385*** Error: Simulation of Buildings.Experimental.DHC.Examples.Cooling.DirectUncontrolledETS failed: 'CalledProcessError: Command '['omc', 'Buildings.Experimental.DHC.Examples.Cooling.DirectUncontrolledETS_simulate.mos']' returned non-zero exit status 1.'. Directory is '/tmp/tmp-Buildings-0-exrbuqkj'. 386Execution time = 1724.656 s 387make: *** [Makefile:145: test-openmodelica] Error 1
Many thanks @JayHuLBL! I'll start working on these. For the errors above, are you able to access the 'simulator-*.log' files to see the reported error details?
Hi @JayHuLBL, I hope you are doing well. Just a friendly follow up on this PR and an update from my end. As of last Friday, I can now successfully run OCT on my workstation.
Do you need anything from me to merge and close this contribution? Many thanks.
@khinkelman I rerun the unit test for the models in package Buildings.Experimental.DHC.Plants.Cooling.Examples
and added the Buildings.Experimental.DHC.Examples.Cooling.DirectUncontrolledETS
to the exclusion list to not run the model with OpenModelica
. I have no further comments.
When the CI tests passed, it should be ready for Michael to review.
I will further process this after https://github.com/lbl-srg/modelica-buildings/pull/3157 is merged to the master (as https://github.com/lbl-srg/modelica-buildings/pull/3157 changes some of the underlying models).
@khinkelman : This is ready to merge when the reference results are updated (which I can do) and when the review comment https://github.com/lbl-srg/modelica-buildings/pull/3093#pullrequestreview-1248749396 is addressed. This goes back to a revision by @ChengnanShi-Work: https://github.com/lbl-srg/modelica-buildings/blame/041de1b9ee39608da70bcaf3205c4ab24c77b7af/Buildings/Experimental/DHC/Loads/Cooling/BuildingTimeSeriesWithETS.mo#L8 Do you or @ChengnanShi-Work know what the rationale is? This should either be removed or explained if it is indeed needed.
@khinkelman : This is ready to merge when the reference results are updated (which I can do) and when the review comment #3093 (review) is addressed. This goes back to a revision by @ChengnanShi-Work: https://github.com/lbl-srg/modelica-buildings/blame/041de1b9ee39608da70bcaf3205c4ab24c77b7af/Buildings/Experimental/DHC/Loads/Cooling/BuildingTimeSeriesWithETS.mo#L8 Do you or @ChengnanShi-Work know what the rationale is? This should either be removed or explained if it is indeed needed.
Thanks @mwetter for your review. I cannot recall the rationale for scaling facMulCoo
in that way. @AntoineGautier, could you please comment here? This setting of facMulCoo=40*QCoo_flow_nominal/(-1.5E5))
came from your Combined.BuildingTimeSeriesWithETS originally, from what I can tell.
These hard-coded numerical values should probably be removed. They come from an example model which was rapidly promoted as a generic class with lack of scrutiny regarding this parameter binding. The current PR #3165 provides a default assignment for facMulCoo
that should rather be used. @dhblum is working on improving the documentation and the code itself which still includes some literal assignments. The rationale is the following.
- A default effectiveness is assigned based on fan coil manufacturer data: this gives the shape of the so-called emission characteristic of the terminal unit Q_flow = f(m_flow).
- The number of terminal units is computed based on the peak load and datasheet capacity.
- An adjustment is made to guard against modifications of air flow or entering temperatures by the user that would be inconsistent with the number of terminal units, i.e., yielding an effectiveness above 1: this is done by adjusting
facMul(Hea|Coo)
at fixed effectiveness. The effectiveness can be modified by manually overwriting the default assignment offacMul(Hea|Coo)
. - What is worth mentioning is that the parameterization always ensures that the specified water dT is achieved at design flows and entering temperatures.
@dhblum Is it best to clean up and merge https://github.com/lbl-srg/modelica-buildings/pull/3165 and then process this PR?
For this PR, I tried to remove the assignment to facMulHea
and facMulCoo
, but then Buildings/Experimental/DHC/Loads/Cooling/Examples/BuildingTimeSeriesWithETS
won't initialize (as the cooling coil design parameters seem to be not meaningful).
Yes, https://github.com/lbl-srg/modelica-buildings/pull/3165 should be merged first. In that PR, the factors in question in https://github.com/lbl-srg/modelica-buildings/pull/3093#issuecomment-1384480665 are effectively moved to defaults within a base model (Buildings.Experimental.DHC.Loads.BaseClasses.Examples.BaseClasses.BuildingTimeSeries) so they won't have to be re-specified in this district cooling model.
If all tests pass, this is ready to be merged.
@dhblum : This is ready to merge if the unit tests succeed. Let me know if I should go ahead or whether you want to do another review.
@dhblum : Thanks for the feedback, I addressed them in the latest commit and will merge when the tests passed.
Sounds good, thanks @mwetter!