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

Issue2048 boiler plant condensation control #2156

Conversation

karthikeyad-pnnl
Copy link
Contributor

No description provided.

@karthikeyad-pnnl
Copy link
Contributor Author

@milicag: Hi Milica, this pull request is ready for review.

Copy link
Contributor

@milicag milicag 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 address my comments.

block CondensationControl
"Sequence to calculate setpoint limits for condensation control in non-condesing boilers"

parameter Boolean primaryOnly = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename to have_priOnl.

parameter Boolean primaryOnly = false
"True: Primary-only plant; False: Primary-secondary plant";

parameter Boolean variablePrimary = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename to have_varPri

"Minimum secondary pump speed";

parameter Real minPriPumSpeSta[nSta](
final unit="1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use final unit=fill("1", nSta) and equivalent edits to all attributes of a vectorized parameter. For now we also use for any such parameters an annotation such as this one: annotation (Evaluate=true);. This annotation will become obsolete once this issue has been resolved and we will do a refactor to remove it then.

"Vector of minimum primary pump speed for each stage";

Buildings.Controls.OBC.CDL.Interfaces.IntegerInput uCurSta
"Current stage"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason not to use uSta?

annotation (Placement(transformation(extent={{100,-20},{140,20}}),
iconTransformation(extent={{100,-20},{140,20}})));

Buildings.Controls.OBC.CDL.Interfaces.RealOutput yMaxSecPumSpe(
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on the conventions please place Min or Max always as a suffix (unless you have a suffix with a leading underscore to use after it, for example _flow for any flow variables.). Here for example you should name the output ySecPumSpeMax.

annotation (Placement(transformation(extent={{-140,-20},{-100,20}}),
iconTransformation(extent={{-140,-20},{-100,20}})));

Buildings.Controls.OBC.CDL.Interfaces.RealOutput yRegSig(
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be only y, as the model name indicates it is a regulator signal?

annotation (Placement(transformation(extent={{-60,-10},{-40,10}})));

Buildings.Controls.OBC.CDL.Continuous.Limiter lim(
final uMax=TRetSet - TRetMinAll,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not permitted in CDL. You could use min/max and have it be an input.

"Minimum secondary pump speed";

parameter Real minPriPumSpeSta[nSta](
final unit="1",
Copy link
Contributor

Choose a reason for hiding this comment

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

again the vectorization of the values assigned to attributes in needed.


Buildings.Controls.OBC.CDL.Continuous.AddParameter addPar3(
final p=1,
final k=(minSecPumSpe - 1)) if variablePrimary
Copy link
Contributor

Choose a reason for hiding this comment

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

@JayHuLBL Please correct me if I am wrong. I believe the translator cannot handle such expressions.

within Buildings.Controls.OBC.ASHRAE.PrimarySystem.BoilerPlant;
package SetPoints


Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove all but one empty line.

@karthikeyad-pnnl
Copy link
Contributor Author

Closing pull request since all sequences will be pulled through #2193.

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.

2 participants