m-lundberg / simple-pid

A simple and easy to use PID controller in Python
MIT License
792 stars 216 forks source link

Separate output limits and integral limits #30

Closed peter-madigan closed 3 years ago

peter-madigan commented 3 years ago

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.

m-lundberg commented 3 years ago

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: https://github.com/br3ttb/Arduino-PID-Library/issues/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.