openems icon indicating copy to clipboard operation
openems copied to clipboard

fix(core): Avoid flickering between charge/discharge due to PID filter

Open parapluplu opened this issue 11 months ago • 19 comments

Before this change the PID filter implementation eventually caused the behavior to switch from charging to discharing, eventhough the system wanted to (a) charge with a lower value than before (b) set active power to 0. This was due to the fact, that the PID filter overshoots its target. By detecting if the system wants charge/discharge/no current flow the battery, it now dynamically adjusts the limits of the battery so it does not overshoot in the unwanted area. This reduces stress on the battery. It also improves the controller that are using this filter (e.g. self consumption optimization, peak load shaving). Sometimes they showed behavior where they wildly flicker between charging/discharging.

Examples: image

self consumption optimization: image

parapluplu avatar Jan 08 '25 14:01 parapluplu

Codecov Report

:x: Patch coverage is 88.23529% with 2 lines in your changes missing coverage. Please review.

Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #2960      +/-   ##
=============================================
+ Coverage      57.09%   57.23%   +0.14%     
- Complexity      9735     9775      +40     
=============================================
  Files           2266     2267       +1     
  Lines          96819    96823       +4     
  Branches        7163     7164       +1     
=============================================
+ Hits           55270    55407     +137     
+ Misses         39487    39348     -139     
- Partials        2062     2068       +6     
:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Jan 08 '25 14:01 codecov[bot]

Looking at your graphs I have some questions:

  1. Is the unit really kW, or is it an error in your Grafana dashboard? Changing from -120kW to 90kW within a few seconds is quite massive.
  2. What does the graph actually show? Is it the active power read from the device (SymmetricEss.ChannelId.ACTIVE_POWER), or the setpoint from OpenEMS (ManagedSymmetricEss.Channeld.DEBUG_SET_ACTIVE_POWER)?
  3. Do you also have "After"-screenshots to see the impact of this PR?

Best regards, Thomas

tsicking avatar Jan 09 '25 10:01 tsicking

Hey,

  1. Yes, it's actually kW.
  2. The graph shows the ActivePower channel of the ess, so SymmetricEss.ChannelId.ACTIVE_POWER, which is the active power of the device.
  3. Partially yes. I have a graph of an inital hot fix I've applied back when I observed the issue. The graph ends, as the next datapoint is a 0 again, which is only send 5 minutes later and therefore you don't see it when you zoom in. The second picture shows the overall picture, but you don't see the PID curves as nicely as in the first one. image image

parapluplu avatar Jan 09 '25 15:01 parapluplu

I've reproduced the problem on a simulated system now to generate graphs for the fix in this merge request (and not just for my previous hotfix). The graphs are much more beautiful, because they show the pure behavior of the PID controller. In the graphs above the hardware factor made the graphs a bit more noisy. :)

Setup: Simulated ESS with max 120kW charge/discharge power. Used self consumption optimization controller. Simulated energy consumption of 150kW, which then suddenly drops to 0kW PID controller with p=0.3, i=0.3, d=0.1

Without the fix you get the following (in my opinion beautiful) graph: image

With the fix you get the following (booooring) graph: image

parapluplu avatar Jan 09 '25 17:01 parapluplu

I can confirm, that i had the same Issues with 0,3 0,3 0,1

Sn0w3y avatar Jan 10 '25 00:01 Sn0w3y

@pooran-c & @huseyinsaht Could you please crosscheck this with our experiences?

sfeilmeier avatar Jan 10 '25 07:01 sfeilmeier

This looks like something I have been seeing in my much smaller system I think. Whenever my system was loaded with a 20kW consumer (Water heater) and the load was suddenly turned off it looked like the system needed some time to balance things out again. When I saw your simulated graph that looked pretty much like something I have been seeing. Not saying it’s the same thing but if it is I can’t wait for this fix being added to FEMS as well. Well done, the after-graph looks great!

TheSerapher avatar Jan 18 '25 21:01 TheSerapher

This looks like something I have been seeing in my much smaller system I think. Whenever my system was loaded with a 20kW consumer (Water heater) and the load was suddenly turned off it looked like the system needed some time to balance things out again. When I saw your simulated graph that looked pretty much like something I have been seeing. Not saying it’s the same thing but if it is I can’t wait for this fix being added to FEMS as well. Well done, the after-graph looks great!

Yes, I also had the same Issues (NO FEMS - native OpenEMS) :)

Sn0w3y avatar Jan 18 '25 22:01 Sn0w3y

Please note that PID is disabled by default for FENECON Home systems, because there we use a specific SMART mode instead, which uses the faster internal reaction time of the inverter.

sfeilmeier avatar Jan 18 '25 23:01 sfeilmeier

Please note that PID is disabled by default for FENECON Home systems, because there we use a specific SMART mode instead, which uses the faster internal reaction time of the inverter.

If that is the case and I am seeing instability when trying to recover from a removed load for a couple of minutes I would assume it’s not working properly? Probably not for this issue then 🙂

TheSerapher avatar Jan 19 '25 07:01 TheSerapher

Quickly threw together a Grafana panel since it just happened again. I think this looks very similar to the graphs above. Purple line (Consumer 3) is a load and after the load is gone it takes quite some time to come to an equilibrium again.

IMG_0197

TheSerapher avatar Jan 19 '25 09:01 TheSerapher

Even though the PID filter may not apply on FEMS systems, the flicker still exists. Here a better view with lower power consumption overall but visible flicker of Grid/Battery fluctuations.

Not sure this is the right place for it since it’s using a different implementation but figured I append the ticket.

IMG_0209 IMG_0208

TheSerapher avatar Jan 29 '25 07:01 TheSerapher

Hi @parapluplu,

We have reviewed your code and noticed that you are manipulating the input of the PID controller. Instead, this should be implemented as a disturbance within the PID controller. pid-controller

Rather than altering the feedback power (input), we should take the calculated power value and then decide how to proceed with it. As we understand, the goal is to observe the output power as shown below (red line). pid-regel

Apart from that, it is quite difficult to predict the outcome because we do not know the model. The best approach is to conceptualize and develop a model for our storage systems. However, this will be discussed with our electrical engineers.

We will give a feedback as soon as possible as we talk to our engineers.

BR, Huseyin Sahutoglu

huseyinsaht avatar Jan 30 '25 11:01 huseyinsaht

Hey @huseyinsaht, thanks for looking into that.

I've a few clarification questions:

We have reviewed your code and noticed that you are manipulating the input of the PID controller.

Where do you see I'm altering the input of the PID controller? Could you make a specific suggestion for a code change? I've not changed anything of the logic of the PID controller itself. The only thing that I change are the limits (which is depending on various factors - like derating, temperature, cell voltage of the battery - an always changing variable). The desired active power is not changed by this pull request.

As we understand, the goal is to observe the output power as shown below (red line). Apart from that, it is quite difficult to predict the outcome because we do not know the model.

If you mean with "goal" to reproduce this issue: yes. The goal of this pull request is to remove this flickering. The model I've used is described in the setup description of https://github.com/OpenEMS/openems/pull/2960#issuecomment-2580883992 and the output is deterministic - But I've the feeling that I'm misunderstanding what you're referring to. Could you please clarify it?

Instead, this should be implemented as a disturbance within the PID controller. [...] The best approach is to conceptualize and develop a model for our storage systems.

Do I get it correct, that you want to solve that problem, by adjusting the PID parameters? Or by changing the PID implementation? I'm wondering how you would avoid a flickering from charging to discharging completely by that. I absolutely see, that it's possible to reduce the flickering, but removing it completely? How do you want to achieve that?

parapluplu avatar Jan 31 '25 12:01 parapluplu

@TheSerapher Your examples are not related to this pull request. This pull request is about changing the active power of the ESS from some positive/negative value to 0 and don't allow any flickering into the opposite direction. Your examples do not show such changes (DC charge energy). Active power remains in positive/negative areas. The only value that's flickering between positive and negative values is the grid power value. However, that behavior is due to your consumer that produces this flickering consumption. Here further tuning of the PID values could improve the behavior - if the PID filter would be used which is not the case as mentioned by @sfeilmeier - but there will always remain some flickering, as we can only adjust the ESS power after measuring the desired value. So there is always some small delay involved. If you want to follow up further about this issue please let's do that in the community forum to keep this pull request focused on the issue about flickering between charging/discharging the ESS.

parapluplu avatar Feb 05 '25 16:02 parapluplu

@parapluplu just to clarify, the consumers 1, 2 and 3 are measured via Shelly, are the only consumers, and do not show any flickering consumption at all. The consumption measured in FEMS flickers until I disable the battery (with a rest command of 0). Then everything is smooth until the battery kicks in again.

I’ll create a forum post tomorrow for this to get some insights 🙂

TheSerapher avatar Feb 17 '25 20:02 TheSerapher

I believe I've observed a similar behaviour with an ESS oscillating between +/- 40kW charging/discharging with load changes in the range of ~10 to 20 kW on default settings. I did some digging myself and I believe this might possibly just be a fix to the symptoms of a bigger underlying problem (not saying it doesn't work though!):

If (and that's a big if) I understood the source code correctly, this is roughly what's happening in a simple setup with a balancing controller for an ESS: Regelkreis OpenEMS drawio

I noticed two things with this:

  1. The feedback value for the actual inverter power cancels out completly, resulting in the system regulating the grid meter with the PID instead of using the actual logic of the balancing controller (the inverter power feedback is positive inside the balancing controller and negative inside the PID filter)

  2. It seems like the PID is regulating the inverter power by setting the power setpoint of the inverter and reading back the actual power. I fail to see the reason for doing this, because the inverter itself already regulates it's output power based on the setpoint and probably does so faster and more accurate than OpenEMS could.

If the goal of the PID filter is to reduce/prevent the oscillations that can happen without it, maybe something like a ramp instead of a PID might be a better solution?

dennis-qt avatar Feb 20 '25 17:02 dennis-qt

This PR has been automatically marked as stale due to inactivity. It will be closed in 7 days if no further activity occurs.

github-actions[bot] avatar Sep 18 '25 02:09 github-actions[bot]

We're running this implementation now in production since the beginning of the year and it still solves the problem.

At the moment I don't understand completely what's the blocker for this merge request

@huseyinsaht Did you discuss that further internally? Also I'm still puzzled about this comment:

We have reviewed your code and noticed that you are manipulating the input of the PID controller.

I do not manipulate the input of the PID controller. I only apply limits. Applying these limits results in the pid filter to change the output, but not the input. For more details how applying those limits affects the PID controller start looking here: https://github.com/OpenEMS/openems/blob/0c705403dffe68578d38edbd9ce21a82bdb98d20/io.openems.edge.common/src/io/openems/edge/common/filter/PidFilter.java#L109

parapluplu avatar Sep 18 '25 14:09 parapluplu