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

Issue2884 pid autotuning #3609

Merged

Conversation

SenHuang19
Copy link

@JayHuLBL Please take a look at the PR based on your comments.

then <code>y = yHig</code>, <code>yOn = true</code>,
</li>
<li>
else if <code>yErr &gt; deaBan</code> and <code>trigger</code> is <code>true</code>,
else if <code>yDif &gt; deaBan</code> and <code>trigger</code> is <code>true</code>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that in current implementation, if yDiff (the input to instance hys) > deaBan, then, hys.y = true, yOn=true and y=yHig. This is opposite to the documentation here.
Which one is right?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for confusion and actually I confused myself with the signs. I have updated the model doc.

@@ -50,23 +50,23 @@ protected
"Lower value for the output"
annotation (Placement(transformation(extent={{-80,20},{-60,40}})));
Buildings.Controls.OBC.CDL.Reals.Subtract conErr
"Control error (set point - measurement)"
"Control error (measurement - set point)"
Copy link
Contributor

Choose a reason for hiding this comment

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

In CDL.Reals.PID and other PID controller variants, the control error is (set point - measurement). See here.
I see that the output yErr from this block becomes input to the block PIDWithAutotuning.SystemIdentification.FirstOrderTimedelayed.ControlProcessModel, so for this block, is it expecting set point - measurement or measurement - setpoint?

Copy link
Author

Choose a reason for hiding this comment

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

I removed the yErr and define a new variable called yDiff. yDiff = yErr if reverseActing is true; otherwise yDiff = -yErr.

@JayHuLBL JayHuLBL merged commit f367eb8 into lbl-srg:issue2884_PID_autotuning Dec 15, 2023
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