search

m-lundberg / simple-pid

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

Derivative Term is calculated on error and not input #39

Closed kmodexc closed 2 years ago

kmodexc commented 3 years ago

the derivative term is calculated on the derivative of the error term and not on the derivative of the input (see here).

The code below should not contain something like dinput but a derror term which should be calculated somewhere.

https://github.com/m-lundberg/simple-pid/blob/15a030fc81f73dc6bfaf8c3084dcf49a415d7e4b/simple_pid/PID.py#L127

m-lundberg commented 3 years ago

Hi!

This is very intentional, please read this guide for an explanation.

Thanks for your input though, I appreciate it!

kmodexc commented 3 years ago

Sorry to bother you again, but isn't it better to make it an option like you did for "proportional_on_measurement"?

My idea would be to make it a choice to use this option. I could write the code for this.

The problem is that indeed if you have a fixed setpoint it is the same but if it is not the case you can have undesirable effect. I want to develop a controller with a linear rising setpoint and my friend just wrote some code with derivative on error having better results than me with your PID library. If you look at the comments below you see that it is very controversial if this is a good idea.

Another thing is that if you use proportional_on_measurement you have basically a redundant parameter.

If you want to use Parameters from Methods like Nichols and Ziegler they would probably only apply to the "normal" version an not on the changed one.

So i would suggest to introduce a parameter differential_on_input which can be set to True or False and works analog to proportional_on_input but for the differetial term.

I appreciate your work! Thanks for that!

m-lundberg commented 3 years ago

It's no bother, I appreciate people who want to help.

Maybe you are right in that the user should have the option. I didn't read the comments before, but it seems that there could be cases where derivative on error is better. Since the intention of this library has always been to be a simple PID controller, I try to be careful with providing too many features that aren't strictly necessary. But in this case, it might be best to give the user the option.

If you want to make a pull request for it, I would appreciate it! Make sure that the current behaviour remains the default though, so that it doesn't become a breaking change for existing users.