Skip to content
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

Issue1337 temperature setpoint partial zone #1391

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

jelgerjansen
Copy link
Contributor

This PR addresses issue #1337.

Copy link
Member

@lucasverleyen lucasverleyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@annadellisola @jelgerjansen thanks for addressing this issue! It's already looking great. I have added a few comments.

@@ -169,11 +176,18 @@ to enable an input for a prescribed boundary condition temperature or heat flow
Alternatively, parameters <code>use_T_fixed</code> and <code>T_fixed</code> can be used
to specify a fixed boundary condition temperature.
It is not allowed to enabled multiple of these three options.
If all are disabled then an adiabatic boundary (<code>Q_flow=0</code>) is used.
If all are disabled then an adiabatic boundary (<code>Q_flow=0</code>) is used.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If all are disabled then an adiabatic boundary (<code>Q_flow=0</code>) is used.</p>
If all are disabled, an adiabatic boundary (<code>Q_flow=0</code>) is used.</p>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit confused with all booleans, i.e. use_T_in and use_T_fixed. It is one or the other (this is specified in the documentation), so I suggest removing one of the two. Also, now the documentation string for both booleans is the same, which is not correct. If one of the booleans is removed, the assert statement can be simplified.
image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently, use_T_fixed and use_T_in have different functions, i.e. for the boundary wall and for the heat design calculation respectively. use_T_in and use_Q_in should be moved to the Advanced > Design power tab.

@@ -64,6 +69,8 @@ protected
IDEAS.Buildings.Components.Interfaces.WeaBus weaBus(final numSolBus=sim.numIncAndAziInBus,
outputAngles=sim.outputAngles) "Weather bus"
annotation (Placement(transformation(extent={{40,-80},{60,-60}})));
initial equation
QTra_design=if use_T_in or use_T_fixed then U_value*A*(TRefZon - T_in_nom) else -Q_in_nom;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TRefZon is determined in the bus connector and equals the reference zone temperature. T_in_nom is a parameter defined in this model and equals T_fixed. But in the case of use_T_fixed = False and use_T_in= True, T_in_nom should be T_in, right?

@@ -504,7 +508,9 @@ end for;
connect(setq50.use_custom_n50s, propsBusInt.use_custom_n50) annotation (Line(points={{-60.8,
-92.8},{-60,-92.8},{-60,-92},{-80.1,-92},{-80.1,39.9}}, color={
255,0,255}));
annotation (Placement(transformation(extent={{
connect(TRefZon.y, propsBusInt.TRefZon);
annotation (Dialog(group="Design heat load", tab="Advanced"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this Dialog(group="Design heat load", tab="Advanced") in the general annotation of the model? Can be removed, I guess.

@@ -41,7 +41,7 @@ protected
if sim.interZonalAirFlowType <> IDEAS.BoundaryConditions.Types.InterZonalAirFlow.None
"Mass flow rate multiplier for port 1"
annotation (Placement(transformation(extent={{-10,-170},{10,-150}})));
Modelica.Blocks.Math.Gain QTra_desgin(k=k) "Design heat flow rate"
Modelica.Blocks.Math.Gain QTra_design(k=k) "Design heat flow rate"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch!

dT_nominal_a=-1,
add_cracks=false,
QTra_design(fixed=false),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QTra_design can currently be modified by the users via the parameter dialogue window. We might set this as a final or protected parameter that the user cannot change this value.
image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment for other components (internal wall, outer wall, ...)

@@ -141,6 +139,8 @@ public
if hasCavity and sim.interZonalAirFlowType == IDEAS.BoundaryConditions.Types.InterZonalAirFlow.OnePort
"1-port model for open door"
annotation (Placement(transformation(extent={{-10,58},{10,78}})));
initial equation
QTra_design=U_value*A*(TRefZon - TRef_b);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TRefZon refers to TRefZon in propsBusA. Maybe a small documentation string might be instructive for other users, explaining that TRef_b is linked to propsBusB and TRefZon to propsBusA.

connect(outsideAir.TDryBul_in, shaType.TDryBul) annotation(
Line(points = {{-42, -90}, {-46, -90}, {-46, -48}, {-58, -48}}, color = {0, 0, 127}));
connect(outsideAir.TDryBul_in, shaType.TDryBul) annotation (
Line(points={{-42,-90},{-46,-90},{-46,-49.4895},{-57.5,-49.4895}},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Line(points={{-42,-90},{-46,-90},{-46,-49.4895},{-57.5,-49.4895}},
Line(points={{-42,-90},{-46,-90},{-46,-48},{-58,-48}},

Ideally, these types of changes are avoided... (at least, when they are not intended).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants