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

Separate output limits and integral limits #30

Conversation

peter-madigan
Copy link

Adds a separate property integral_limits to clamp the integral term independently from the output.

This fixes an issue I encountered where the pid does not stabilize to the setpoint when using output_limits with an integrating system (a PWM heater that I control with a 0->1 float). In this case, the proportional-on-measurement term becomes larger than the clamped integral term, and the pid output goes to 0 - very far from the setpoint. So, I think it would be nice to be able to set the output_limits separately from the wind-up limiting on the integral term.

I've provided an example that demonstrates this behavior below

import time
import simple_pid

pid = simple_pid.PID(Kp=0.1,Ki=0.1,Kd=0.1,setpoint=1000,output_limits=(0,1),proportional_on_measurement=True)

class IntegratingSystem(object): 
    def __init__(self): 
        self.state = 0

    def __call__(self, input_): 
        self.state += input_ - 0.1
        return self.state

input_ = 0
system = IntegratingSystem() 
while True:
    input_ = pid(system(input_))
    print(input_, system.state)
    time.sleep(0.1) 

This PR fixes this by adding an integral_limits property that is set in the same way as the output_limits. The integral_limits are applied solely on the integral term, whereas the output_limits are applied solely on the output.

Adds a separate property ``integral_limits`` to clamp the integral term independently from the output.
@m-lundberg
Copy link
Owner

Hello,

First of all, thanks for taking the time to create your pull request. I appreciate the effort!

The question of separating output_limits and integral_limits has come up before for this library. I chose then to not introduce separate limits for a couple of reasons. The main thing is that I don't really see why it would be necessary (though I could be wrong; I'm not an expert in control theory), and I think the added complexity would be a bad thing for this library which aims to be a simple PID implementation (it's in the name).

For your particular example I still don't see why separate limits would be necessary. I got it to converge quite nicely to the setpoint by changing the tunings to Kp=0.0001, Ki=0.1, Kd=0.01. It could probably be improved more by messing around with the tunings even further. Remember that when using proportional on measurement, the P-term acts as a resisting force rather than a driving one, so if the proportional term "resists too much" you need to set a lower Kp.

There has also been some related discussion for the Arduino PID library that inspired this library to begin with, you can check it out here: br3ttb/Arduino-PID-Library#76 . If they reach a different conclusion there I might reconsider at some point, but for now I think keeping just one set of limits is "good enough".

There is also a different perspective on this that needs to be taken into consideration. Separating the limits at this point would be a breaking change so it would at least need to go into a new major release. Even then it could be easy for people to upgrade the library version and suddenly find that their controller is no longer behaving correctly. This would need to be handled somehow if I ever decide to make this change.

If you still don't agree, I might be willing to reconsider in the future, but for now I'm closing this.

@m-lundberg m-lundberg closed this Mar 20, 2021
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