Issue2180 boiler plant main controller oct 2021 #2700
Conversation
| </p> | ||
| <p align=\"center\"> | ||
| <img alt=\"Validation plot for EfficiencyCondition1\" | ||
| src=\"modelica://Buildings/Resources/Images/Controls/OBC/ASHRAE/PrimarySystem/BoilerPlant/Staging/SetPoints/Subsequences/Down1.png\"/> |
There was a problem hiding this comment.
@karthikeyad-pnnl These plots are missing.
|
@karthikeyad-pnnl I corrected the html errors, but did not get chance to check if all the changes from #2685 are included here. At least I noticed some images are missing. |
…ler_oct_2021 Issue2180 boiler plant main controller oct 2021
…ntroller_oct_2021 Issue2180 boiler plant main controller oct 2021
…MainController_oct_2021 Issue2180 boiler plant main controller oct 2021
|
@karthikeyad-pnnl
|
There was a problem hiding this comment.
@karthikeyad-pnnl Please see the inline comments that I have after working on the integration of the G36 boiler plant controller into the boiler plant template developed in the feature branch issue3266_template_HW_plant of MBL.
Please note that this is not a full review: those comments only pertain to the parameters and I/O points of the controller. (The validity of the control logic implementation was not reviewed.)
| parameter Integer nIgnReq( | ||
| final min=0) = 0 | ||
| "Number of hot-water requests to be ignored before enabling boiler plant loop" | ||
| annotation(dialog(tab="Plant enable/disable parameters")); |
There was a problem hiding this comment.
Incorrect annotation (several occurrences): dialog should be capitalized.
| parameter Real schTab[nSchRow,2] = [0,1;6,1;18,1;24,1] | ||
| "Table defining schedule for enabling plant" |
There was a problem hiding this comment.
- Shouldn't it rather be a software point (input connector), and not a parameter? Particularly it should accommodate weekdays and holidays.
- Missing consistency with
issue2293_chiller_plant_seqwhere the schedule time values are provided insas opposed tohhere. - Since
size()is a function allowed in CDL (https://obc.lbl.gov/specification/cdl.html#functions) why exposenSchRowas a parameter as opposed to assigning it simply withfinal parameter Integer nSchRow = size(schTab, 1)?
| parameter Integer nPumSec | ||
| "Total number of secondary hot water pumps" | ||
| annotation (Dialog(group="Boiler plant configuration parameters")); | ||
|
|
||
| parameter Integer nSenSec | ||
| "Total number of remote differential pressure sensors in secondary loop" | ||
| annotation (Dialog(group="Boiler plant configuration parameters")); | ||
|
|
||
| parameter Integer nPumSec_nominal( | ||
| final max=nPumSec) = nPumSec | ||
| "Total number of pumps that operate at design conditions in secondary loop" | ||
| annotation (Dialog(group="Boiler plant configuration parameters")); |
There was a problem hiding this comment.
Should be enabled only for primary-secondary plants.
| parameter Boolean have_priOnl = false | ||
| "Is the boiler plant a primary-only, condensing boiler plant?" |
There was a problem hiding this comment.
How are primary-only non-condensing boiler systems supported?
| parameter Boolean have_secFloSen=false | ||
| "True: Flowrate sensor in secondary loop; | ||
| False: Flowrate sensor in decoupler" |
There was a problem hiding this comment.
The description of the false condition is confusing as primary-secondary plants may have neither a flow sensor in the secondary loop nor in the decoupler (and rely on Delta-T for primary pump control).
| parameter Real maxLocDpPri( | ||
| final unit="Pa", | ||
| displayUnit="Pa", | ||
| final quantity="PressureDifference", | ||
| final min=1e-6) = 5*6894.75 | ||
| "Maximum primary loop local differential pressure setpoint" | ||
| annotation (Dialog(tab="Primary pump control parameters", group="DP-based speed regulation", | ||
| enable = speConTypPri == Buildings.Controls.OBC.ASHRAE.PrimarySystem.BoilerPlant.Types.PrimaryPumpSpeedControlTypes.localDP | ||
| or speConTypPri == Buildings.Controls.OBC.ASHRAE.PrimarySystem.BoilerPlant.Types.PrimaryPumpSpeedControlTypes.remoteDP)); |
There was a problem hiding this comment.
This comment pertains to this declaration and the following one:
parameter Real maxLocDpSec(
final unit="Pa",
displayUnit="Pa",
final quantity="PressureDifference",
final min=1e-6) = 5*6894.75
"Maximum hot water loop local differential pressure setpoint in secondary loop"
- I would use only one parameter instead of two as it is actually the setpoint of the same sensor, i.e., the differential pressure needed at the boundaries of the distribution system to serve the loads.
- How to justify a default value (generic) for those parameters? If provided anyway it should differ from the minimum setpoint value of 5 psi.
| Buildings.Controls.OBC.CDL.Interfaces.BooleanOutput yBoi[nBoi] | ||
| "Boiler status vector" |
There was a problem hiding this comment.
"Status" is the current On/Off state of a device as reported by the hardware itself, so a DI point.
The description string should rather be: "Boiler enable signal".
| Buildings.Controls.OBC.CDL.Interfaces.BooleanOutput yPla | ||
| "Plant enable signal" |
There was a problem hiding this comment.
Not an output point per G36 4.11. Should be removed.
| Buildings.Controls.OBC.CDL.Interfaces.BooleanOutput yPriPum[nPumPri] | ||
| "Primary pump enable status vector" |
There was a problem hiding this comment.
See comment above about "status": replace with "signal".
| Buildings.Controls.OBC.CDL.Interfaces.RealOutput TPlaHotWatSupSet( | ||
| final unit="K", | ||
| displayUnit="K", | ||
| final quantity="ThermodynamicTemperature") | ||
| "Plant hot water supply temperature setpoint" |
There was a problem hiding this comment.
- Not an output point per G36 4.11. Should be removed.
- (Temperatures should rather have `displayUnit="degC".)
This replaces #2685
@JayHuLBL @karthikeyad-pnnl: Please make sure this contains the changes from #2685
Note that this is on a branch with the new name
issue2180_BoilerPlant_MainController_oct_2021to distinguish it from the old branchissue2180_BoilerPlant_MainController(whose changes are on this branch too).I essentially did the following: