Open Mathadon opened 1 year ago
For IDEAS.Examples.PPD12.VentilationRBC
, hFloor=0
for all zones. Shouldn't the zones of the first and second floor have an adapted floor height in order to account for the stack effect?
For
IDEAS.Examples.PPD12.VentilationRBC
,hFloor=0
for all zones. Shouldn't the zones of the first and second floor have an adapted floor height in order to account for the stack effect?
@kldjonge can you comment on this one?
@jelgerjansen I have addressed the comments (except zone icon layout) in 6900db4f35b320fef3749b15f6560ce0a5bb48ac
For
IDEAS.Examples.PPD12.VentilationRBC
,hFloor=0
for all zones. Shouldn't the zones of the first and second floor have an adapted floor height in order to account for the stack effect?@kldjonge can you comment on this one?
Indeed. That is one of the things that were changed on the other branch. But feel free offcourse to change this allready.
@kldjonge am I correct that this height will only affect the results of the 2 port configuration? Because for PPD12 a 1-port configuration is used. Might be better to add this change PR1323, otherwise it might be more difficult to compare the reference results (too many effects that are playing).
Indeed, it should only affect the 2 port config. IF it changes the results in the 1-port implementation, there is is bug.
Then let’s keep all absolute heights at 0 for this PR. If there is still a bug in the 1-port configuration, we might detects those in PR1323.
Currently some models fail to simulate which should be fixed. An overview (models and errors):
IDEAS.Buildings.Components.Examples.WindowThermalBridge
: Overriding final modifier for len. (fixed by commit 3773c4a)IDEAS.Buildings.Examples.ScreenComparison
: The sum of the screen transmittance, reflectance, and absorptance does not equal one. (fixed by commit 99fae2102)IDEAS.Examples.PPD12.VentilationRBC
: error somehow induced by combination of sim(interZonalAirFlowType=IDEAS.BoundaryConditions.Types.InterZonalAirFlow.OnePort)
and airModel(massDynamics=Modelica.Fluid.Types.Dynamics.SteadyState)
. (fixed by commit 3f4ac1a58)IDEAS.Examples.TwinHouses
: Overriding final modifier for len. (fixed by commit 416191f)IDEAS.Examples.TwinHouses.BuildingO5_Exp1_1Port
: struct.W40.resDoor.m_flow
does not exist any more. (fixed by commit 5ac5f7bdc)IDEAS.Examples.TwinHouses.BuildingO5_Exp1_1Port
: Structurally singular error (fixed by commit 3f4ac1a58)Furthermore, the Airflow package was not included in the unit tests. This change should first be merged in the master branch (#1331), after which the master branch can be merged in this PR:
The unit test IDEAS.Buildings.Validations.Tests.n50Test2
stops at time=2 seconds due to the following error (using branch open-ideas:project_itz_fixes):
Error: The following error was detected at time: 2
n50 computation consistency check failed
Failed condition: abs(ach1-zone1.n50) < 1E-06 or time < 2
Disabling the assert shows that zone1.n50
is slightly higher compared to ach1
Comparing the results with those of the master branch shows that the mass flow rate through the window has increased (0.00179341 kg/s to 0.00210989 kg/s), explaining the higher n50 value. The origin of this problem lies (I think) in the area that is used in PropsBusInt: this is the area read from layMul
which is equal to A_glass
, but for airflow the area of the entire window A
should be used. Both areas differ with a factor 2/1.7 difference, explaining the simulated difference in airflow through the window.
Below you can find the new/updated reference. Most updates seem to be minimal, but for IDEAS.Examples.TwinHouses
the results indicate that something went wrong when going from resDoor.m_flow
to crackOrOperableDoor.m1_flow
.
IDEAS.Airflow
IDEAS.Buildings.Components
IDEAS.Buildings.Examples
IDEAS.Buildings.Validation
IDEAS.Examples.PPD12
IDEAS.Examples.Tutorial
IDEAS.Examples.Tutorial.Example9
takes way too long. I've raised a seperate issue to solve this (#1336)
IDEAS.Examples.TwinHouses
Is this ready to be merged?
@jelgerjansen I have added a minor change that I spotted. Problem is that I can't get the models to simulatie with OM to test it myself.
Purple/orange should be the same but are very different and their sign is opposite. This suggests that something is wrong with the driving forces of the flow.
Flow rate seems to be in line with pressure drop:
Absolute pressure difference in bathroom:
Atmospheric pressure in green.
matches
Pressure distributions of zones:
W40-W42 has a cavity and should have no substantial pressure dorp. doo.y=1
for these walls.
COpe = 0.388
for all doors. This equals CD
for the old model.
CVal=0.466 in old model, whereas CVal=COpe in new model.
Pressure drops of the doors don't have a different order of magnitude somehow:
16 Pa difference:
while 8 Pa difference on each column so something external seems to be generating the pressure difference:
Okay, root cause seems to be similar flow rates for very different pressure drops:
CVal is nearly but not exactly the same: 0.388
in new implementation vs 0.466
in old implementation.
Despite the large differences, dpAB
is actually quite similar:
See:
parameter Real hAg[nCom](each unit="m2/s2")=
{Modelica.Constants.g_n*(hA - (i - 0.5)*dh) for i in 1:nCom}
"Product g*h_i for each compartment";
parameter Real hBg[nCom](each unit="m2/s2")=
{Modelica.Constants.g_n*(hB - (i - 0.5)*dh) for i in 1:nCom}
"Product g*h_i for each compartment";
equation
pA[i] = port_a1.p + rho_a1_inflow*hAg[i];
pB[i] = port_a2.p + rho_a2_inflow*hBg[i];
dpAB[i] = pA[i] - pB[i];
However the model has
final hA = 0,
final hB = 0,
so these equations should not have a big effect.
The problem seems to be that we are externally applying a stack instead of letting the door model handle it. This would have been fine if the model equations were consistent, but they are not: the external pressure column uses hVertical
instead of the door height hOpe
.
The cleanest approach would be to refactor the model such that we used the internal stack computation again.
I have pushed some changes and tested them on ZoneExample
but OM cannot run TwinHouse. Can you have another look whether the results for the model have improved?
E.g. the mass flow rate for W1 on this figure:
@Mathadon the results seem to be better (solid line represents the results of PR #1322, dashed line the results of this PR)
When zooming in, both results show exactly the same trend, but there is still a minor offset of about (2e-5 for W1[1], 8e-5 for W7[1], 2e-4 for W10, and 1e-7 for W37). The latter might be due to the change CVal/COpe?
The problem seems to be that we are externally applying a stack instead of letting the door model handle it. This would have been fine if the model equations were consistent, but they are not: the external pressure column uses
hVertical
instead of the door heighthOpe
.The cleanest approach would be to refactor the model such that we used the internal stack computation again.
Isn't the external column their is a door/opening doesn't start on the floor level? (e.g. a large opening between a living room and a kitchen that only starts above the cabinets..). I'm not familiar with the specific changes you made when combining/optimizing the models, but that was the approach before I believe. If I recall correctly, the door model assumes the pressure in the connecor to be the reference at the bottom of the door, so their should be some kind of correction based on the room heights (and preferably the 'starting height' of the door for e.g. modelling a 2-storey hallway with as 1-zone).
@kldjonge to be clear, the problem that I described is a problem that I created by modifying your implementation. As far as I know, the equations are now again consistent with the old implementation.
The way I see it, the external column is now integrated into the door model but I did not look into the model equations in full detail to verify their correctness. Since the implementation is the same as before, it should be equally correct as before and the validation should point out whether or not that the results match contam and thus whether the implementation is correct.
@jelgerjansen the CVal/COpe should be correct since commit aad7465.
Perhaps the remaining differences are because the new implementation effectively disables the cracks when a door is open. In the old implementation both equations would be present at the same time. The underlying assumption was that the crack flow rate is negligible to the flow rate of an open door. Looking at the values of CClo=5e-5
and COpe=0.388
this should indeed be the case. The differences seem larger than that. So probably it's something else.
@jelgerjansen would it be possible to send me those two .mat files such that I can compare further?
At least part of the reason seems to be that the q50
values are not the same. E.g. for W1 q50=0.910208
vs q50=0.883414
. This is the same ratio as the mass flow rate mismatch.
The cause of this change is probably:
+ setArea(A=A*nWin),
- setArea(A=A_glass*nWin),
in the Window
model, for which the new implementation is the more accurate computation as far as I am concerned.
So I think we can accept these changes and move ahead..! @kldjonge @jelgerjansen do you agree?
Is the difference in ratio A/A_glass
the same as the ratio of both q50
values? But probably this does not have to be like this because the q50 value is a combination of all wall components connected to that zone?
Before moving on, I would like to merge this PR and redo all unit tests to make sure that nothing else broke while fixing the issue of the TwinHouses example.
@jelgerjansen Indeed, the results are affected by all surfaces so the q50 value does not scale proportionally.
I would propose to update the reference results of the current PR and then run unit tests again for https://github.com/Mathadon/IDEAS/pull/2 to look for new changes.
@Mathadon I'll update the reference results of the current PR, but I'll compare them to those of the stack airflow branch (#1322) to do one last check of this PR.
I've included an overview of the reference results that show significant changes compared to those of commit . The reference results are those from #1322.
IDEAS.Buildings.Components
There is a large change in U_default
, which is caused by a higher heat flow. Not sure if this can be attributed to the changes made in the last couple of commits.
IDEAS.Examples.TwinHouses
IDEAS.Examples.TwinHouses.BuildingO5_Exp1_1Port
still shows a relatively high difference between W40[1]/W37[1] and W40[1]_ref/W37[1]_ref (so compared to the reference results of #1322). Maybe this should be looked into before continuing.
IDEAS.Examples.TwinHouses.BuildingO5_Exp1_2Port
now has the same reference results as #1322, which seems good. However, we must not forget to include the airflow throug wall W40 once the validation data of the 2-port configuration using CONTAM is availabe @kldjonge.
@jelgerjansen commit 07c74e447e5cfc27424184b6e11371b41c636d70 fixes a parameter bug that is a possible cause for the remaining changes. I pushed it on branch project_itz_fixes_v2.
@Mathadon I ran the branch project_itz_fixes_v2, but I ran into an error which I solved in commit 33e525ce6 (see branch origin/project_itz_fixes_v2).
Unfortunately, I get the same 'problems': a large deviation of U_default for IDEAS.Buildings.Components.Validations.WindowEN673
and comparison_w40[1] for IDEAS.Examples.TwinHouses.BuildingO5_Exp1_1Port
The deviation of U_default
in IDEAS.Buildings.Components.Validations.WindowEN673
is due to a change of the window fraction (and therefore A_glass
): the default value of 0.15 for the component window
was used previously, but is now changed to 0. This new value seems more appropriate since it is also used in the other window component windowEN673
.
Therefore, the updated the reference results (commit 69d307eab) are the 'correct' ones.
For clarity, we still have to cover the comments here: https://github.com/Mathadon/IDEAS/pull/2
@jelgerjansen this includes some changes but still requires a master merge