-
Notifications
You must be signed in to change notification settings - Fork 163
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 #2957
base: master
Are you sure you want to change the base?
Issue2884 pid autotuning #2957
Conversation
Buildings/Controls/OBC/CDL/Continuous/Validation/AMIGOWithFOTD.mo
Outdated
Show resolved
Hide resolved
Buildings/Controls/OBC/CDL/Continuous/Validation/AMIGOWithFOTD.mo
Outdated
Show resolved
Hide resolved
Buildings/Controls/OBC/CDL/Continuous/Validation/AMIGOWithFOTD.mo
Outdated
Show resolved
Hide resolved
Buildings/Controls/OBC/CDL/Continuous/Validation/PIDWithAutoTuning.mo
Outdated
Show resolved
Hide resolved
Buildings/Controls/OBC/CDL/Continuous/Validation/PIDWithAutoTuning.mo
Outdated
Show resolved
Hide resolved
Buildings/Resources/Scripts/Dymola/Controls/OBC/CDL/Continuous/Validation/PIDWithAutoTuning.mos
Outdated
Show resolved
Hide resolved
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: This requires considerable more work. Please also see the email I sent.
@SenHuang19 Run the model |
@JayHuLBL Before merging the master branch, the results appear to be fine. I will thoroughly review it. |
|
Issue2884 pid autotuning
The CI test is failing with error as below:
|
The error with the license check occasionally occurs when the license manager takes too long to start and in the meantime Dymola proceeded without checking out the license. Restarting the CI tests helps. |
@mwetter Thanks! It now passed the test. I will have another look and then let you know if it is ready for your review. |
@mwetter It's ready for your review. |
…ses a PI not a P controller
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 @JayHuLBL :
This PR still has a few issues. First, note that I deleted the parameter u_s_start
as it has no effect. The start value is overridden as soon as the tuning is initiated. So I set the start value of the sampler to 0.
- A minor thing to address is the default value for
yHig
andyLow
. The parameteryHig
has a default of1
butyLow
has no default. This is inconsistent. Is there a rational for it? If so it needs to be documented. - The documentation of
yLow
andyHig
needs to be improved. It is not clear what they are used for. - The "guidance for setting the parameters" is not clear. This needs to be made clearer.
- The main issue is that the example
WetCoilCounterFlowPIControlAutoTuning
, formerly calledWetCoilCounterFlowPControlAutoTuning
, does not work: The autotuning is interrupted and the message isAutotuning failed. The controller gains are unchanged
. The only change I think compared to the older version is that the stroke time of the valve is now 120 s (the default) and not 240 s. Changing the stroke time is not a design choice in HVAC control. This change was somewhat accidental as I introduced a base class, but I have not seen a valve with a stroke time larger than 120 s. I am surprised why a faster process fails the tuning, as it should be easier to control. This hints towards some bug for which there needs to be an explanation and/or a better example.
Thank you so much for the edits and comments. I will create a PR to address the issues. |
@mwetter @JayHuLBL The tuning failure is due to a symmetric response from the controlled system. An asymmetric response is typically easier to achieve during the transient period at the start of the simulation. When the valve stroke time is 240s, the transient period is longer, resulting in an overlap with the tuning period. In this case, the tuning completes successfully. However, when the valve stroke time is reduced to 120s, this overlap no longer occurs, causing the tuning to fail. I attempted to adjust the |
This merges the PID autotuning to the master.