ButterFlight / butterflight

GNU General Public License v3.0
106 stars 26 forks source link

"Buttered Pids" are equivalent to "Classic Pids" in the absence of Setpoint Weight and Dterm Notch #162

Closed ghost closed 6 years ago

ghost commented 6 years ago

This unit test has been built to prove that "buttered pids" are no different than "classic pids" when the dterm notch is disabled setpoint weight is set to zero.

The additional code and configuration is superfluous bloat and adds no additional value. If you feel differently, please write a test that proves otherwise.


[----------] Global test environment set-up.
[----------] 8 tests from ButteredPidsHypeTest
[ RUN      ] ButteredPidsHypeTest.NoSetpointWeightAndNotch_SP
[       OK ] ButteredPidsHypeTest.NoSetpointWeightAndNotch_SP (75 ms)
[ RUN      ] ButteredPidsHypeTest.NoSetpointWeightAndNotch_NOSP
[       OK ] ButteredPidsHypeTest.NoSetpointWeightAndNotch_NOSP (89 ms)
[ RUN      ] ButteredPidsHypeTest.NoSetpointWeightAndNotch_SP_LPF
[       OK ] ButteredPidsHypeTest.NoSetpointWeightAndNotch_SP_LPF (90 ms)
[ RUN      ] ButteredPidsHypeTest.NoSetpointWeightAndNotch_NOSP_LPF
[       OK ] ButteredPidsHypeTest.NoSetpointWeightAndNotch_NOSP_LPF (97 ms)
[ RUN      ] ButteredPidsHypeTest.FailOnSetpointWeight_SP
[       OK ] ButteredPidsHypeTest.FailOnSetpointWeight_SP (0 ms)
[ RUN      ] ButteredPidsHypeTest.FailOnSetpointWeight_NOSP
[       OK ] ButteredPidsHypeTest.FailOnSetpointWeight_NOSP (0 ms)
[ RUN      ] ButteredPidsHypeTest.FailOnNotch_SP
[       OK ] ButteredPidsHypeTest.FailOnNotch_SP (0 ms)
[ RUN      ] ButteredPidsHypeTest.FailOnNotch_NOSP
[       OK ] ButteredPidsHypeTest.FailOnNotch_NOSP (0 ms)
[----------] 8 tests from ButteredPidsHypeTest (352 ms total)```
rs2k commented 6 years ago

ackchyually, Kd is using the measurement method of calculation, not the error method of calculation, so your unit tests are wrong, bro.

ghost commented 6 years ago

Can you please comment in the code to prove your assessment? Responses without justification are just responses.

ghost commented 6 years ago

Simply following the order of mathematical operations (something a 3rd grader can do) proves that there is no difference.

rs2k commented 6 years ago

This site explains it rather well: https://controlguru.com/pid-control-and-derivative-on-measurement/

Control is not factored into D-term's calculation. It provides for a much smoother flight experience.

ghost commented 6 years ago

I sincerely want to be proven wrong. I challenge anyone to do so.

ghost commented 6 years ago

By supplying a setpoint weight of 0, control is removed from the calculation and is left only with gyro measurements. By removing the notch, it left only with the LPF. This makes them exactly the same.

rs2k commented 6 years ago

control is factored into PID error, actually.

rs2k commented 6 years ago

error calculation looks kind of like this:

            kdDelta[axis] = pidError - lastfilteredGyroData[axis];
            lastfilteredGyroData[axis] = pidError;

whereas measurement calculation looks kind of like this:

            kdDelta[axis] = -(filteredGyroData[axis] - lastfilteredGyroData[axis]);
            lastfilteredGyroData[axis] = filteredGyroData[axis];

The output differs significantly, but ONLY when control input is introduced, which is my guess as to why your unit test is probably failing to produce different results.

rs2k commented 6 years ago

Kd calculation using measurement does NOT use PID error, it simply looks at the gyro data. That's why they differ.

ghost commented 6 years ago

Based on the conditions above, the derivative is based on the measurement and stick deflection has no influence. I have posted comments in the code that prove the unit tests are valid. Please comment to disprove them if you feel they are in error.

rs2k commented 6 years ago

I think you just proved why buttered PIDs are a great idea. That's a lot of unnecessary code there when measurement is the best method. :)

ghost commented 6 years ago

These tests are not designed to verify superiority of control. They are designed to disprove that "buttered pids" do nothing more that could already be accomplished using existing means. Pushing the agenda that this code does something different and is somehow superior is misleading to the community and proof that quality peer reviews are not of priority in Butterflight.

rs2k commented 6 years ago

Or maybe we like to toy with 13 hour old accounts coming out of lefht field. ;)

ghost commented 6 years ago

I created this account to join github and add to the community the findings that these tests prove. Up to this point I have not desired to contribute, so I have not had an account. I fail to see how the age of my account casts any doubt on these findings.

rs2k commented 6 years ago

Probably because of all the trolls. :)

If you're truly interested in helping please join butterflight and/or helio's slack and we can talk there. We're always happy to work with others, but we aren't going to take trolls or anything resembling a troll very seriously.

ghost commented 6 years ago

I have tried to join the slack but see nothing but people trolling other flight controller software. I have no desire to join a community forum where moderators are tolerant of childish behavior.

ghost commented 6 years ago

I might also add, that if you don't take trolls very seriously, your team should take a long look in the mirror and reflect upon the behavior of your inner circle. Some of the developer/moderators are exhibiting very troll-like behaviors and are not impressing upon the public a feeling of professionalism.

L0fty commented 6 years ago

It looks like we stopped talking about code. I think we can close this up.

rs2k commented 6 years ago

I agree with @L0fty. I'm closing this PR for now.

orneryd commented 6 years ago

I finally got a chance to look at this PR and while they may be equivalent in certain cases, the user experience is terrible. Nobody of the general public understands what those values actually do. also, modifying the D calculations directly based on stick position is a terrible way to go about trying to find equivalence against a simpler calculation that uses less CPU, less flash, and introduces less latency to the control loop.

In the end what actually matters is: a) how correctly the quad flies and b) how easy it is to get your quad flying correctly

dterm setpoint weight and transition are useless tools meant to compensate for poor understanding of 100 year old control theory. As per our argument this whole time, "you should never adjust pid values for stick feel. that what rates are for." let the pids do their job. use the correct tool for the job you are trying to do. it simplifies things drastically.

butterSnake commented 6 years ago

img_0257

orneryd commented 6 years ago

Mousedist!