Klipper3d
Update temperature_fan.py to have working PID option
Whereas the bang-bang control works as expected (fan turns on when measured temp is > reference temp), the ControlPID as designed doesn't have the same behavior and the duty cycle is max_speed - PID value, which is not the expected behavior.
The correction allows the PID to work correctly.
As the minimum fan speed is generally about 20%, a tolerance equal to max_delta has been programmed to avoid that the fan oscillate between 0 and min_speed near the reference value. It only kick-in when the measured temperature is >reference temperature + max_delta.
The temp_err variable is < 0 when the fan should power-on and if the value is lower, the fan must speed up, so all PID coefficients must be negative. (or the temp_err should be inverted).
Used successfully to control the radiator fan of a water-cooled printer
I honestly don't understand why temperature_fan has PID control, PID control is most useful when there is direct control over the process variable (the temperature in this instance). I understand the idea of speeding up the fan the hotter the temp, but with PID the fan speed will just constantly "hunt" and the integral portion will constantly build up an error that makes it slow to respond to changes.
Case in point, Lets say I'm using temperature fan on my mcu to cool down the main processor. It's completely possible that no matter how fast my fan is going, if my mcu is maxed out in it's processing it will heat up to a point above the set point no matter what the fan does. By the PID logic, it will look at this and say "I'm not trying hard enough" and it will continue to push that integral sum higher and higher because the error will constantly be off. Then when the mcu load drops, the fan won't drop speed very quickly because the integral term will be so high it thinks it needs to way over compensate.
I can only think of two ways to fix this...
1.) Make the fan a PD (Proportional-Derivative) control and ditch the integral altogether. 2.) Use a fan curve, which is probably more simple for 90% of users anyways and could be visually added to Mainsail/whatever UI is being used.
That's just my two cents. The temperature_fan setup has always been janky to me.
Edit: Thinking about this a little more, The issue with using a single set point for the temp is that normally the temps are pretty variable. I think a band of temps would better fit user expectations and is another plus for the fan curve.
In other words, If I'm using the temp fan to cool off one of my mcus and I put my set point at 50c. When my mcu is over 50c (not uncommon) my fan is howling cause it's running full speed.
What I'd PREFER is.. If the temp was 30-40c run at 30% power, if it's 41-60c run at 60% power, if it's over 60% run at max speed. Or something along those lines. Of even a smooth curve ramped up to a certain point. I don't believe a PID controller gives that functionality.
I totally agree... My goal was simply to have a silent fan... the code was there and wrong, now it isn't wrong. Up to anybody to choose the PID parameters to make it a P or P-D control...
I will update the doc accordingly.
The only problem I see with this approach is, generally you want to get AHEAD of cooling. If you turn the fan off as soon as it hits the set point you've effectively removed cooling and it's likely to heat back up to hit that +1 c point again and turn back on and thus oscillate around that point. Generally you want to try to gradually slow down cooling to avoid an immediate heat up again.
I can understand maybe adding in where it turns off at a certain minimum point. As in, maybe adding in an optional parameter to the gcode command to have a "minimum temp shutoff" or something similar. Say having a set point of 50c and a min shut off of 30c so the fan spools down and if it gets down to 30c just shut off altogether.
Also I looked into the temp fan more and I guess they do have a way to prevent integral windup so that takes care of one of my concerns. PID still feels like a clunky and overly complex way to go about this, but that's neither here nor there for this discussion.
In my case, the 1°C offset means a lot of energy: There is about 200 cc of water to heat up and cool down, plus the radiator, pump housing...etc. It's different if the fan is used to cool something with a low thermal mass and a fast temperature change.
My goal is not to have a constant temp... I just need a silent cooling fan to keep my temp within reasonable limits with variable heat sources.
This offset could become a parameter, if necessary, for specific purpose. For a water cooling system, 1°C is exactly what I needed to have the fan at a constant 20% duty as long as heat is produced by the head and extruder motor. Without this offset, it oscillates permanently between 0 and 20%.
Obviously, if the goal is to respect a precise temperature, a more complex strategy might be needed by taking directly into account the power supplied to the heat sources and anticipate the cooling accordingly, but this is another story...
I definitely understand your use case, but you have to understand the bigger picture with the pull request.
Specifically here... Contributing to Klipper
Part 2 under "What to expect during a review"
"Does the submission provide a "high impact" benefit to real-world users performing real-world tasks?"
The overwhelming majority of 3d printer users don't use water cooled printers. For them, this change would be confusing because the fan would just turn off when it hits their set point which isn't what is wanted in an air cooled setup (What probably 99.8% of users have).
That being said, I run a custom version of klipper with my own tweaks that I know other people might not like. I know a lot of people do. So having your own version that has tweaks for your setup isn't unheard of if you just want to go that route. But almost always main line additions to Klipper have to benefit that vast majority, if not all, users. Or on the inverse, implement something that a lot of users might use but doesn't break functionality for those that don't want to use it.
If you really want to get this into the main klipper repository, I'd suggest re-writing it with the "min temp shutoff" config adjustment. This would satisfy your use case, would potentially be useful to other users (I'd use it), and wouldn't break any existing functionality.
Basically, what is questionable is the use of a PID with a fan: The fan control is proportional only within its speed range and the minimum voltage it needs to run is at least 15% of the nominal value... Now the question is how to manage that.
- As described in the doc, the temperature fan is supposed to be activated if the temperature is above the reference temperature, and is supposed to lower this temperature when activated. This is working only if the reference temperature is above the temperature of the displaced air due to the fan action (which is not necessarily the air going through the fan). A more generic control system must take this into account and integrate this second temperature in the computation, and maybe have an option to control the temperature delta instead of an absolute value...
- The present code has a strange output: bang-bang control is logical: On if too hot, off if T < T ref.
- PID is now for a situation where the fan is at maximum duty cycle when the temperature is near the reference and is decreasing when the temperature difference increase... Is it supposed to be a more generic behavior than what I'm trying to propose? I can't see any situation where it is useful.
- My version has at least the same logic as the bang-bang it is supposed to improve.
- As long as the heat source power is higher than the cooling power of the fan at the minimum rpm, it should not oscillate, at least if the temperature probe is at the right place. There is no way to control every possible system with any possible temperature probe position with a simple code, and it is logical that the fan is stopped until the monitored temperature is higher than T ref, and has a suitable (and maybe parametric) dead zone around Tref to avoid that the fan switches on every second when the monitored temperature is at Tref.
I'm by no means the final approver here, just bringing up the concerns I'm sure are to arise. While I agree with all your criticisms, the thing that stands out to me is this...
My version has at least the same logic as the bang-bang it is supposed to improve.
That's what will be confusing, because people expect it to be somewhat PID controlled but it will act like a bang bang controller.
The expectation (though the existing code doesn't do this well, or at all sometimes) is that the fan will wind down speed to a minimum once the temperature is below it's set point value.
As long as the heat source power is higher than the cooling power of the fan at the minimum rpm, it should not oscillate
The problem is, in your code you're not using the minimum RPM, you're setting the speed to zero. So it will oscillate, as soon as it hits the set point it will turn off and when it's 1c higher it will turn back on.
if temp_err>0: # stop the fan if the temp is lower than the target temp
spd = 0.
else:
if temp_err<-1: #enable the fan when the temp diff. is >1deg
spd = max(self.temperature_fan.get_min_speed(),bounded_co)
I regularly watch my temps bounce around, it might be a slow oscillation, but it will be an odd oscillation. If you add more hysteresis we're right back to a bang bang controller.
Again, I'm with you on the PID control being weird in temp_fan. After thinking about it even more, I think it should just be purely proportional, which goes right back to a fan curve. Then have a "min_temp_cutoff" point for people to use like in your use case.
That's easier to understand than saying something is PID but it doesn't behave like a PID controller, which is what this update will do. The concern is more about circumventing the PID logic without making it a new thing altogether, the user expectation is PID, janky or not, when it doesn't behave like a PID controller at all there will be a million github issues describing it as a problem.