Skip to content

Issue2180 boiler plant main controller oct 2021#2700

Open
mwetter wants to merge 1677 commits intomasterfrom
issue2180_BoilerPlant_MainController_oct_2021
Open

Issue2180 boiler plant main controller oct 2021#2700
mwetter wants to merge 1677 commits intomasterfrom
issue2180_BoilerPlant_MainController_oct_2021

Conversation

@mwetter
Copy link
Copy Markdown
Member

@mwetter mwetter commented Oct 27, 2021

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_2021 to distinguish it from the old branch issue2180_BoilerPlant_MainController (whose changes are on this branch too).

I essentially did the following:

 git checkout -b karthikeyad-pnnl-issue2180_MainBoilerPlantController issue2180_BoilerPlant_MainController
 git checkout -b issue2180_BoilerPlant_MainController
 git pull
 git branch --set-upstream-to=origin/issue2180_BoilerPlant_MainController issue2180_BoilerPlant_MainController
 git pull
 git diff --name-only origin/master
 git checkout -b karthikeyad-pnnl-issue2180_MainBoilerPlantController issue2180_BoilerPlant_MainController
 git pull https://github.com/karthikeyad-pnnl/modelica-buildings.git issue2180_MainBoilerPlantController
 git checkout issue2180_BoilerPlant_MainController
 git merge --no-ff karthikeyad-pnnl-issue2180_MainBoilerPlantController
 git diff --name-only origin/master
 git checkout -b issue2180_BoilerPlant_MainController_oct_2021
 git diff --name-only origin/master
 git push --set-upstream origin issue2180_BoilerPlant_MainController_oct_2021

</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\"/>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@karthikeyad-pnnl These plots are missing.

@JayHuLBL
Copy link
Copy Markdown
Contributor

@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.

@JayHuLBL
Copy link
Copy Markdown
Contributor

@karthikeyad-pnnl Would you please let me know if the sequences have been updated using the updated CDL blocks?

@JayHuLBL
Copy link
Copy Markdown
Contributor

@karthikeyad-pnnl Would you please run following commands to check the text formatting and html syntax? See below:

  • run ../bin/verifyFiles.py to check format:

Screen Shot 2022-09-22 at 2 26 40 PM

  • run ../bin/runUnitTests.py --validate-html-only

Screen Shot 2022-09-22 at 2 28 05 PM

  • please run the unit test to see if there is any results change. And if yes, check if the changes are expected.

  • remove the redundant zeros in the .mos script, see 48e38f6.

  • check if there is any BOM being introduced, see 7effeca. You could remove the BOM as here.

@JayHuLBL
Copy link
Copy Markdown
Contributor

@karthikeyad-pnnl
Some classes are broken due to the refactoring of the CDL block, see #3128.

  • fix the broken classes
  • re run unit tests and update reference results.

Copy link
Copy Markdown
Contributor

@AntoineGautier AntoineGautier 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 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.)

Comment thread Buildings/Controls/OBC/ASHRAE/PrimarySystem/BoilerPlant/Controller.mo Outdated
Comment thread Buildings/Controls/OBC/ASHRAE/PrimarySystem/BoilerPlant/Controller.mo Outdated
Comment thread Buildings/Controls/OBC/ASHRAE/PrimarySystem/BoilerPlant/Controller.mo Outdated
Comment thread Buildings/Controls/OBC/ASHRAE/PrimarySystem/BoilerPlant/Controller.mo Outdated
Comment thread Buildings/Controls/OBC/ASHRAE/PrimarySystem/BoilerPlant/Controller.mo Outdated
Comment thread Buildings/Controls/OBC/ASHRAE/PrimarySystem/BoilerPlant/Controller.mo Outdated
Comment thread Buildings/Controls/OBC/ASHRAE/PrimarySystem/BoilerPlant/Controller.mo Outdated
Comment thread Buildings/Controls/OBC/ASHRAE/PrimarySystem/BoilerPlant/Controller.mo Outdated
Comment thread Buildings/Controls/OBC/ASHRAE/PrimarySystem/BoilerPlant/Controller.mo Outdated
This was referenced Apr 25, 2023
@karthikeyad-pnnl
Copy link
Copy Markdown
Contributor

@AntoineGautier I have made the following updates in the latest push to #4319. Please review it when you get a chance. Thanks!

  1. Flow-regulation valve: The flow-regulation valve operates independent of the hot-water plant, and is responsible for generating the requests that enable the plant. Because of the limits used to generate the plant request (1 request when valve exceeds 95%, 0 when it falls below 10%), the valve will always open to 95% before the requests are generated and the plant switches on, at which point the flow is always going to be a high value. The use of a PI controller with reset would not change this behavior significantly, since the valve position signal will only be reset when the plant and pumps are enabled, and the valve would still have to physically travel from 95% to the reset value, which would follow a similar traversal rate as the current scenario.
  2. Primary pump control: The only flowrate parameter that appears to be mandatory across multiple boiler plant configurations with flowrate-based control is the minimum boiler flowrate. I have accordingly used it for the scaling value.
  3. Sizing parameters and documentation: I have simplified the sizing parameters in the example model, and removed most of the redundant defaults from the baseclasses. I have also added extensive notes about the sizing process and usage to the documentation section, rather than the individual instance comments. I have parametrized the pressure value calculations and setpoints to the max extent I could. Finally, documentation also contains notes about the validation plots.
  4. Parameter selections, Boolean enable conditions, and parameter comments: I have revisited the parameter declarations for the variable speed primary pumps, secondary loop flowrate sensor and the primary loop temperature sensor.

PS: Is there any convenient way to resolve the merge conflicts caused by the commits directly made here? Or should I just manually resolve them on my working branch?

@AntoineGautier
Copy link
Copy Markdown
Contributor

AntoineGautier commented Oct 19, 2025

@karthikeyad-pnnl Putting more thought into the validation model, I think the load model should be revisited and that the heating heat flow rate should be tracked, rather than the HW mass flow rate. The reason is that, contrary to the chiller plant, there is no ∆p reset for the boiler plant, only a supply temperature reset. So, at low load, the control intent is to reset down the HWST to maximize the opening of the terminal control valves. Controlling for a mass flow rate taken from another simulation makes little sense because, as you noticed, at low load the plant gets disabled because the terminal control valves close. If you switch to using, e.g., an explicit radiator model and track the heating heat flow rate, the HWST reset at low load keeps the control valve more open and the simulation exhibits the actual control intent, which is to operate at low temperature and high flow rate. The plant only gets disabled at zero load. I've quickly tested this in https://github.com/AntoineGautier/modelica-buildings/tree/tmp/issue3266_template_HW_plant: please have a look and let me know what you think.

Can you also rename the validation model into Guideline36, or at least suppress Test from the model name?

Also, can you change the size of the plots to match the one of other validation models in the library? Below is how the plots are displayed on a my system.

image

As for the merge conflicts, best is to resolve them locally by merging issue2180_BoilerPlant_MainController_oct_2021 into your development branch. (When doing that, and since there has been a lot of back and forth during this development, it's usually useful to check all the files that differ from the master branch and do some necessary cleaning.)

@AntoineGautier
Copy link
Copy Markdown
Contributor

@karthikeyad-pnnl Following up on my previous comment regarding the load model, the cleanest would be to use Buildings.Fluid.HydronicConfigurations.ActiveNetworks.Examples.BaseClasses.LoadTwoWayValveControl.
This is now the class used in the template validation model Buildings.Templates.Plants.Boilers.HotWater.Validation.BoilerPlant from 73ec89a.

@karthikeyad-pnnl
Copy link
Copy Markdown
Contributor

@karthikeyad-pnnl Following up on my previous comment regarding the load model, the cleanest would be to use Buildings.Fluid.HydronicConfigurations.ActiveNetworks.Examples.BaseClasses.LoadTwoWayValveControl. This is now the class used in the template validation model Buildings.Templates.Plants.Boilers.HotWater.Validation.BoilerPlant from 73ec89a.

@AntoineGautier Thanks, Antoine! Hope to wrap this and the merge conflicts up this week

@karthikeyad-pnnl
Copy link
Copy Markdown
Contributor

@karthikeyad-pnnl Putting more thought into the validation model, I think the load model should be revisited and that the heating heat flow rate should be tracked, rather than the HW mass flow rate. The reason is that, contrary to the chiller plant, there is no ∆p reset for the boiler plant, only a supply temperature reset. So, at low load, the control intent is to reset down the HWST to maximize the opening of the terminal control valves. Controlling for a mass flow rate taken from another simulation makes little sense because, as you noticed, at low load the plant gets disabled because the terminal control valves close. If you switch to using, e.g., an explicit radiator model and track the heating heat flow rate, the HWST reset at low load keeps the control valve more open and the simulation exhibits the actual control intent, which is to operate at low temperature and high flow rate. The plant only gets disabled at zero load. I've quickly tested this in https://github.com/AntoineGautier/modelica-buildings/tree/tmp/issue3266_template_HW_plant: please have a look and let me know what you think.

Can you also rename the validation model into Guideline36, or at least suppress Test from the model name?

Also, can you change the size of the plots to match the one of other validation models in the library? Below is how the plots are displayed on a my system.

image As for the merge conflicts, best is to resolve them locally by merging `issue2180_BoilerPlant_MainController_oct_2021` into your development branch. (When doing that, and since there has been a lot of back and forth during this development, it's usually useful to check all the files that differ from the master branch and do some necessary cleaning.)

@AntoineGautier Thanks for pointing out the load baseclass in the HydronicConfigurations package. I was able to get the described behavior at low-load conditions. I have also made the other requested changes on #4319. Please review them when you get a chance. Thanks!

@AntoineGautier AntoineGautier marked this pull request as ready for review January 30, 2026 15:35
Copy link
Copy Markdown
Contributor

@AntoineGautier AntoineGautier left a comment

Choose a reason for hiding this comment

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

That looks good.
I've updated the load model and reference results.

  • It would be good to understand and eliminate the warnings below — non blocking though
    Warning: The following was detected at time: 86400
    In Guideline36.conBoiPri.staSetCon.staDow.extIndSig: The extract index is out of the   range.
    With: index=0
    Failed condition: conBoiPri.staSetCon.staDow.extIndSig.index > 0 and conBoiPri.staSetCon.staDow.extIndSig.index <= 3
    
  • Could you fix the CI tests issues

And that would be ready!

@karthikeyad-pnnl
Copy link
Copy Markdown
Contributor

That looks good. I've updated the load model and reference results.

  • It would be good to understand and eliminate the warnings below — non blocking though
    Warning: The following was detected at time: 86400
    In Guideline36.conBoiPri.staSetCon.staDow.extIndSig: The extract index is out of the   range.
    With: index=0
    Failed condition: conBoiPri.staSetCon.staDow.extIndSig.index > 0 and conBoiPri.staSetCon.staDow.extIndSig.index <= 3
    
  • Could you fix the CI tests issues

And that would be ready!

@AntoineGautier I have summarized the information about the multiple warnings below. It has to do with the use of the RealExtractor block in the various control modules. When the modules were being setup, the design relied on the then-existing parameter to define the output when the index was outside a pre-defined set of bounds. The feature was primarily used in calculations related to the plant and pump staging by outputting a zero signal when the plant was disabled, indicated by stage 0 (this is a placeholder and not related to the other discussion about stage 0 operation in the boiler plant). Once the feature was removed from the CDL block, the various modules were refactored to restore the previous functionality with the use of the Buildings.Controls.OBC.ASHRAE.G36.Plants.Boilers.Generic.ZeroIndexCorrection block as required. The warnings persist in the latest implementation of the RealExtractor block, and their root cause is summarized below. During the refactoring process, the modules' performance was verified to ensure no impact on design intent.

Instance Time instances when issue is reported (Each instance reported is when unique state is achieved for the signal. Not exhaustive) Index input signal Root cause
conBoiPri.minBoiFloSet1[3].extIndSig4 index > 0 and index <= 3 86400 Staging setpoint+1 Signal is constantly 4 because flow setpoint is calculated for next higher stage (in this case stage-4 doesn't exist and impact calculations)
conBoiPri.staSetCon.staDow.extIndSig index > 0 and index <= 3 86400; 89019.32597823304; 91717.65026802519; 94163.49447427988; 169693.3543601964; 256052.8632797867 Staging setpoint Signal is 0 when plant is disabled
conBoiPri.upProCon.hotWatSupTemRes.extIndSig index > 0 and index <= 3 86400; 88833.32597823304; 91531.65026797933; 93977.4944742799; 169507.3543601116; 255866.8632797867 Staging setpoint Signal is 0 when plant is disabled
conBoiPri.hotWatSupTemRes.extIndSig index > 0 and index <= 3 86400; 88833.32597823304; 91531.65026797933; 93977.4944742799; 169507.3543601116; 255866.8632797867 Staging setpoint Signal is 0 when plant is disabled
conBoiPri.staSetCon.staDow.extIndSig1 index > 0 and index <= 3 86400; 89019.32597823304; 91717.65026802519; 94163.49447427988; 169693.3543601964; 256052.8632797867 Staging setpoint Signal is 0 when plant is disabled
conBoiPri.priPumCon.lasLagPum index > 0 and index <= 2 86400; 89016.32597823306; 91714.65026797933; 94160.4944742799; 169690.3543601116; 256049.8632799148 No. of pumps enabled Signal is 0 when all primary pumps are disabled
conPumSec2.nexLagPum index > 0 and index <= 1 87304.50000004354; 89737.82597827794; 92436.15026797939; 94881.99447432737; 188813.3519835187 No. of pumps enabled+1 Signal is 2 when secondary pump is enabled
conPumSec1.nexLagPum index > 0 and index <= 1 87304.50000004354; 89737.82597827794; 92436.15026797939; 94881.99447432737; 188813.3143210534 No. of pumps enabled+1 Signal is 2 when secondary pump is enabled
conBoiPri.priPumCon.nexLagPum index > 0 and index <= 2 104342.4372401533; 105726.5416235401; 107711.8596648953; 121963.1802918463; 124652.6738330294; 195804.6139936947; 196988.4998158395; 198981.9686468744; 201077.8358555062; 203884.0470173713 No. of pumps enabled+1 Signal is 3 when all primary pumps are enabled

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.

4 participants