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

Issue2885 fan coil unit controls #2946

Conversation

karthikeyad-pnnl
Copy link
Contributor

@JayHuLBL : I am creating this PR for the fan coil unit controls. I have currently assigned the destination branch as the 'master' branch. Can you please create a new development branch "issue2885_FanCoilUnitControls" on lbl-srg/modelica-buildings? I will then edit the PR to use that as the target branch.

@JayHuLBL
Copy link
Contributor

@karthikeyad-pnnl I just created the branch issue2885_FanCoilUnitControls for this PR. You can rebase the PR now.

@karthikeyad-pnnl karthikeyad-pnnl changed the base branch from master to issue2885_FanCoilUnitControls March 31, 2022 18:06
@karthikeyad-pnnl
Copy link
Contributor Author

@karthikeyad-pnnl I just created the branch issue2885_FanCoilUnitControls for this PR. You can rebase the PR now.

Thanks!

@karthikeyad-pnnl
Copy link
Contributor Author

karthikeyad-pnnl commented Nov 30, 2022

@JayHuLBL: @terrancelu92 and I have updated the files according to the previous comments. Please see my responses in bold for each of the comments. Thanks!

@karthikeyad-pnnl Please merge the latest base branch to your branch.

See the comments below:

  • in FanCoilUnit.Subsequences.FanSpeed, rename the parameter heaSpeMin, heaSpeMax to heaSpe_min, heaSpe_max. The intention is to make the min/max parameter to have the surfix _min, _max.
    Files changed accordingly.
  • in FanCoilUnit.Subsequences.FanSpeed, rename the group name Heating loop parameters to Heating loop. Also, you may combine the Deadband parameters and Transition parameters to Deadband. Change the comments of the heaDea to Heating loop signal limit above which it changes from deadband mode to heating mode.
    Files changed accordingly.
  • in FanCoilUnit.Subsequences.FanSpeed, change the instance name isOcc to notUno.
    Files changed accordingly.
  • in FanCoilUnit.Subsequences.FanSpeed, change the parameter name heaPerMin, heaPerMax to uHea_min, uHea_max, as well as the cooling loop parameters.
    Files changed accorddingly.
  • in FanCoilUnit.Subsequences.FanSpeed, if fan is not proven on (uFanPro=false) and it is not in unoccupied operation mode, the fan speed will be one (yFanSpe=1)?
    Updated fan speed logic so that fan speed is at minimum until it is proven on
  • in FanCoilUnit.Subsequences.PlantRequests, the parameters like chiWatResReqLim3, as we can directly use the value specified in the G36, such like chiWatResReqLim3, it is 5.56. In the info section, we can write it as 10 F (5.56 C).
    Documentation updated accordingly
  • in FanCoilUnit.Subsequences.PlantRequests, it has the check if it is in cooling max or heating max mode. But it did not document in the info section. It seems not being specified in the G36?
    The check is for fan at max cooling speed, which is specified in section 5.22.8.1.a in G36.
  • in FanCoilUnit.Subsequences.SupplyAirTemperature, rename the parameter heaPerMin to uHea_min.
    Files changed accordingly
  • in FanCoilUnit.Subsequences.SupplyAirTemperature, rename THeaSupAirHi and TCooSupAirHi to THeaSup_max and TCooSup_min, rename TAirSupSet to TSupSet.
    Files changed accordingly.
  • in FanCoilUnit.Subsequences.SupplyAirTemperature, in info section, add space Similarly,TAirSupSet
    Files changed accordingly.
  • in FanCoilUnit.Subsequences.SupplyAirTemperature, rename the TZonSetHea to TZonHeaSet
    Files changed accordingly.
  • in FanCoilUnit.Subsequences.SupplyAirTemperature, in the info section, the diagram plot, please make sure the parameter names are consistent with the names in the code. For example, in the plot, it has THeaSupAirHi, while in code, it is THeaSupAirMax.
    Images and files changed accordingly.
  • in FanCoilUnit.Subsequences.SupplyAirTemperature, we used two PID controllers for heating and cooling coil. Is it possible to use one controller, but change the min, max value to [-1, 1] and then do conversion to positive value, depending on yHeaCoi, yCooCoi?
    My thought process was to use the two separate PI controllers to provide separate tunable parameters for the heating and cooling coils, since the dynamics of the heating and cooling actions may be different. Are we using the method you proposed anywhere else in the Buildings library?
  • in FanCoilUnit.Subsequences.SupplyAirTemperature, in the info section, uCoo and uHea are set to zero when ..., should it be yHeaCoi and yCooCoi are set to zero when ...?
    Documentation changed accordingly.

@JayHuLBL
Copy link
Contributor

@karthikeyad-pnnl

  • In FanCoilUnit.Controller, the following connectors rename and the comments should be passed to the subsequences
    • change the output connector yFan to y1Fan and change the comment to Fan command on status,
    • change the output connector yFanSpe to yFan and change the comment to Fan command speed,
    • change the input uFanPro to u1FanPro
    • change uOcc to u1Occ, uFan to u1Fan, uWin to u1Win

@karthikeyad-pnnl
Copy link
Contributor Author

@JayHuLBL We have updated the interface instances according to the above comments. Please review the changes when you get a chance. Thanks!

"Subsequence for calculating supply air temperature setpoint"

parameter Boolean have_cooCoi
"Does the fan coil unit have a cooling coil? True: Yes, False: No";
Copy link
Contributor

@JayHuLBL JayHuLBL Nov 29, 2023

Choose a reason for hiding this comment

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

change the comment to True: the unit has a cooling coil, and give it a default value true

"Does the fan coil unit have a cooling coil? True: Yes, False: No";

parameter Boolean have_heaCoi
"Does the fan coil unit have a heating coil? True: Yes, False: No";
Copy link
Contributor

Choose a reason for hiding this comment

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

True: the unit has a heating coil


Buildings.Controls.OBC.CDL.Interfaces.RealOutput yCooCoi(
final unit="1",
displayUnit="1") if have_cooCoi
Copy link
Contributor

Choose a reason for hiding this comment

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

The yCooCoi and yHeaCoi are conditional connectors. However, the conditions are not propagated upstream. For example, the instances mul3, swiCooCoi, cooPIDCoo can be conditional removed if have_cooCoi=false. so that the parameters needed in instance cooPIDCoo can be disabled.

The conditions to diable parameters may need to be propagated to Buildings.Controls.OBC.ASHRAE.FanCoilUnit.Controller.

equation
connect(uHea, linTHeaSupAir.u) annotation (Line(points={{-140,40},{-74,40},{-74,
60},{-62,60}}, color={0,0,127}));

Copy link
Contributor

Choose a reason for hiding this comment

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

remove the empty lines.

"Fan speed setpoint subsequence"

parameter Boolean have_cooCoi
"Does the fan coil unit have a cooling coil? True: Yes, False: No";
Copy link
Contributor

Choose a reason for hiding this comment

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

update the comment and set the default value to true.

Also, please propagate the condition to instances and parameters.

Copy link
Contributor

@JayHuLBL JayHuLBL left a comment

Choose a reason for hiding this comment

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

@karthikeyad-pnnl Please see the inline comments. There are following html errors:

jianjunhu@ubuntu:~/GitFolder/karthikeyad-pnnl/modelica-buildings/Buildings$ ../bin/validateHTML.py 
The following malformed html syntax has been found:
[-- ./Controls/OBC/ASHRAE/FanCoilUnit/Subsequences/Validation/PlantRequests.mo ]
line 55 column 1 - Warning: inserting implicit <p>
line 25 column 1 - Warning: trimming empty <p>
line 55 column 1 - Warning: trimming empty <p>

@JayHuLBL JayHuLBL merged commit 16f5c4b into lbl-srg:issue2885_FanCoilUnitControls Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

5 participants