-
Notifications
You must be signed in to change notification settings - Fork 157
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
Issue2884 pid autotuning #3590
Issue2884 pid autotuning #3590
Conversation
@@ -93,7 +93,9 @@ block FirstOrderAMIGO | |||
Buildings.Controls.OBC.Utilities.PIDWithAutotuning.Relay.Controller rel( | |||
final yHig=yHig, | |||
final yLow=yLow, | |||
final deaBan=deaBan) "Relay controller" | |||
final deaBan=deaBan, | |||
reverseActing=reverseActing) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add final
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
{-70,-6},{-70,26},{-62,26}}, color={0,0,127})); | ||
connect(conErr.u2, u_s) annotation (Line(points={{-62,14},{-90,14},{-90,0},{-120, | ||
0}}, color={0,0,127})); | ||
if reverseActing then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should not have the if else
in the equation section. We only allow the connect
statement.
To enable or disable the connection, you can add a CDL.Reals.MultiplyByParameter
instance (with k=1
) and make the instance conditional. This will enable/disable the connection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made two components for comparing the reference and the measurement, which are enabled/disenabled based on reverseActing
and removed the if else
in the equation section.
@@ -131,15 +147,17 @@ boolean relay switch output <code>yOn</code>, and the control error | |||
</p> | |||
<ul> | |||
<li> | |||
<code>yErr = u_s - u_m</code>, | |||
<code>yErr = u_m - u_s</code>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to change the documentation as:
- if the parameter
reverseActing = false
, then,yErr = u_m - u_s
,- if yErr < -deaBan and trigger is true, ...
- if ...
- ...
- else if the parameter
reverseActing = true
, then,yErr = u_s - u_m
,- if yErr < -deaBan and trigger is true, ...
- if ...
- ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As suggested, the model doc is updated.
@@ -0,0 +1,150 @@ | |||
within Buildings.Controls.OBC.Utilities.PIDWithAutotuning.Validation; | |||
model DirectActingPIDWithFirstOrderAMIGO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may change the class name in the validation package as:
DirectActingPID
, DirectActingPI
, ReverseActingPID
, ReverseActingPI
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As suggested, I changed the names but keep WithFirstOrderAMIGO
as other system models may be added in the future.
@@ -0,0 +1,215 @@ | |||
within Buildings.Fluid.HeatExchangers.Examples; | |||
model WetCoilCounterFlowPControlAutoTuning |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SenHuang19 Please see inline comments.
@JayHuLBL could you please kindly take a look at it?