Have statistics functions return a meaningful, non-none result even if only one value is available

[unfug-at-github]

Have statistics functions return a meaningful, non-none result even if only one value is available

Breaking change

The statistics sensor will now calculate a value based on a single numerical sample from the input sensor, instead of requiring two numerical samples, for the following characteristics:

• average_linear
• average_step
• distance_95_percent_of_values
• distance_99_percent_of_values
• noisiness
• percentile
• standard_deviation
• sum_differences
• differences_nonnegative
• variance

Proposed change

Currently many of the statistics function return none when only one value is available, however, a meaningful value exists (e.g. the average of { 17.5 } is 17.5). This change will return a value for all of those cases. None will still be returned in cases where no value is available.

This is primarily an issue when you have values that are (at least sometimes) not changing frequently. E.g. my solar production is noisy during the day (meaning subsequent measurements will usually vary at least a little), however, at night the value is stable (zero). E.g. when I want to compute the average, the maximum, the minimum of those values, it will return none at night. This value will become none at night even though everything is working fine and it could (and should) report zero in the mentioned cases and you have to build workarounds around it to combine the current value with the average to have the "real average" computed.

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Deprecation (breaking change to happen in the future)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request:

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• I have followed the perfect PR recommendations
• The code has been formatted using Ruff (ruff format homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[home-assistant]

Hey there @ThomDietrich, mind taking a look at this pull request as it has been labeled with an integration (statistics) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of statistics can trigger bot actions by commenting:

• @home-assistant close Closes the pull request.
• @home-assistant rename Awesome new title Renames the pull request.
• @home-assistant reopen Reopen the pull request.
• @home-assistant unassign statistics Removes the current integration label and assignees on the pull request, add the integration domain after the command.
• @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
• @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

[emontnemery]

@unfug-at-github Can you please motivate why your proposed change is beneficial?

[eljoest]

I'm not the author of the PR but a user, so please accept my apologies if you didn't want to hear my opinion.

I've noticed significant issues with statistical values, especially when home assistant is restarted and/or the sensor that provides values for the statistical entity is "sparse" and does not provide many values but only (significant) changes (e.g. to conserve energy).

I hope for a speedy inclusion of this PR and the other, linked one, as I see spikes and incorrect values from the statistics entities after a lot of restarts of home assistant as well as the issues described in the other PR when a sensor's value does not change for a long time (e.g. "0" illuminance at night from a sensor but the average not returning to zero, constant temperature values that break derivative calculations).
After a reboot two days ago my "average outdoor temperature" sensor bounced to several thousand degrees below zero, back to a few thousand above and only recovered to correct values after more temperature changes were reported by the zigbee sensors.

[unfug-at-github]

@unfug-at-github Can you please motivate why your proposed change is beneficial?

Please have a look at the description. I have updated it with a corresponding scenario where the current behavior is an issue.

[emontnemery]

@eljoest @unfug-at-github I am not convinced this PR does anything to improve the issues you mention since the statistics sensor restores past states of input entities from the recorder?

[unfug-at-github]

@eljoest @unfug-at-github I am not convinced this PR does anything to improve the issues you mention since the statistics sensor restores past states of input entities from the recorder?

Yes, that is what it does, assuming those changes have happened within the the specified interval (max_age). It doesn't load values from the database that are older than max_age.

This leads to the buffer being filled with the values that have changed within the interval. If there is only a single value there, the "average_step" / "average_linear" becomes "unknown" instead of that value.

[eljoest]

@emontnemery For the jumps: Nope, this will not change anything - that's what the other request is supposed to address.

Most of my sensors are zigbee based and only send new values when there's a significant change. Unfortunately they're sometimes "noisy" so I use statistics entities with "keep_last_sample" to smooth them. In periods with constant conditions the recorder will only have a single value (or no value at all) within the max_age timeframe and this seems to lead to issues. I haven't debugged this after the addition of keep_last_sample so my conclusions may be outdated but the issue is still present.

[unfug-at-github]

@emontnemery For the jumps: Nope, this will not change anything - that's what the other request is supposed to address.

Most of my sensors are zigbee based and only send new values when there's a significant change. Unfortunately they're sometimes "noisy" so I use statistics entities with "keep_last_sample" to smooth them. In periods with constant conditions the recorder will only have a single value (or no value at all) within the max_age timeframe and this seems to lead to issues. I haven't debugged this after the addition of keep_last_sample so my conclusions may be outdated but the issue is still present.

You are exactly right. Currently many of the functions return "unknown" when there is only a single value. This means that even if I use "keep_last_sample", and hence, have a single value, the result is "unknown".

By the way, even if I use "keep last sample" I am not able to properly compute the statistics of the last minute. Assuming my sensor provides 0.0 (1 hour ago) and then 1.0 (1 second ago) and I compute the minimum of the last minute. The buffer would only hold the 1.0, since "keep last sample" only applies when there is no value within the interval. The outcome of this would be:

minimum of last minute = 1.0 (instead of 0.0)
average_step of last minute = unknown (instead of 1/60 = 0.1666)

I have already solved all these issues, but was asked to split the pull requests into small portions. So this one just fixes the issues around the computation failing for buffers containing only a single value.

Just have a look at the following example to see how messy averages currently look with input that is perfectly fine to compute averages.

# Loads default set of integrations. Do not remove.
default_config:

# Load frontend themes from the themes folder
frontend:
  themes: !include_dir_merge_named themes

template: 
  - trigger:
      - platform: time_pattern
        seconds: "/2"
    sensor:
      - name: "xxx_test_float"
        unit_of_measurement: 'x'
        state_class: measurement
        availability: "true"
        state: >   
          {% set now = now() %}
          {% set secs = now.second + (60 * now.minute) %}
          {% set modsecs = secs % 90 | int %}
          {% if (secs % 2) == 1 %}
            {% set secs = secs-1 %}}
          {% elif modsecs <= 10 %}
            {{ modsecs }}
          {% elif modsecs <= 30 %}
            10
          {% elif modsecs <= 40 %}
            {{ 40 - modsecs }}
          {% elif modsecs <= 60 %}
            0
          {% elif modsecs <= 70 %}
            10
          {% else %}
            0
          {% endif %}

sensor: 
  - platform: statistics
    name: "xxx2_test_avg_step_10s_refresh"
    entity_id: sensor.xxx_test_float
    state_characteristic: average_step
    max_age:
      seconds: 10
  
  - platform: statistics
    name: "xxx2_test_avg_linear_10s_refresh"
    entity_id: sensor.xxx_test_float
    state_characteristic: average_linear
    max_age:
      seconds: 10
 
logger:
  default: info

[emontnemery]

@unfug-at-github i still lack an explanation of how this PR improves the behavior considering the statistics sensor restores past states of input entities from the recorder

Could you maybe try to dig down in the git history (the easiest way would be to use git blame, either in github or in vscode) to see what the rationale was for only calculating most characteristics when there are at least 2 states?

Why do some of Python's stdlib statistics functions raise when called with a single value? The statistics library was added https://bugs.python.org/issue18606, I could not find an explanation to why it was chosen to raise when the number of samples is < 2 for some of the functions.

[unfug-at-github]

@unfug-at-github i still lack an explanation of how this PR improves the behavior considering the statistics sensor restores past states of input entities from the recorder

Could you maybe try to dig down in the git history (the easiest way would be to use git blame, either in github or in vscode) to see what the rationale was for only calculating most characteristics when there are at least 2 states?

Why do some of Python's stdlib statistics functions raise when called with a single value? The statistics library was added https://bugs.python.org/issue18606, I could not find an explanation to why it was chosen to raise when the number of samples is < 2 for some of the functions.

The simple answer to the first question is: it doesn't. There is a separate change request to address the issues of the spikes.

Regarding the other topic: I can only speculate why several of the functions don't return a value for the single value case. Most likely it wasn't considered because the authors had sensors spitting out enough values that were changing frequently (being noisy). Also the whole thing started with limiting the buffer by size not by it's age, so the problem arose only after "max_age" was introduced. I can only say that computing an average, a maximum, a mean etc. are mathematically well defined for the single value case, and it doesn't make any sense at all that this module is unable to compute those values.

[eljoest]

i still lack an explanation of how this PR improves the behavior considering the statistics sensor restores past states of input entities from the recorder

The recorder will not restore more values to satisfy "arbitrary" limitations of the statistics functions (e.g. average_step needing two data points to calculate the average).

Could you maybe try to dig down in the git history (the easiest way would be to use git blame, either in github or in vscode) to see what the rationale was for only calculating most characteristics when there are at least 2 states?

I did to the original PR ~ 3 years ago. Other than the fact that the current implementation can't compute the value with only one point I was unable to identify a conscious choice. As @unfug-at-github mentioned there are still edge cases (e.g. with one single value within the max_age frame and another value just outside) that can not be adressed with this PR. As already stated the other reviewer requested that the larger PR be split into smaller pieces. As far as my understanding of both PRs goes, doing so unfortunately results in partial/incomplete fixes of the current issues.

Why do some of Python's stdlib statistics functions raise when called with a single value? The statistics library was added https://bugs.python.org/issue18606, I could not find an explanation to why it was chosen to raise when the number of samples is < 2 for some of the functions.

Not sure which stdlib statistics functions do so, but I suspect in these cases there are mathematical rules in place that define it's impossible.

Depending on the statistic the solution would likely be to provide more than the last sample (e.g. the last two) - which I think was part of the original PR. This would also be required to fix the example edge case mentioned before, but cannot be accomplished in a "small" and minimal PR as requested.

[ThomDietrich]

Hey guys.

Could you maybe try to dig down in the git history (the easiest way would be to use git blame, either in github or in vscode) to see what the rationale was for only calculating most characteristics when there are at least 2 states?

Most likely it wasn't considered because the authors had sensors spitting out enough values that were changing frequently (being noisy).

Happy to explain my reasoning for this decision some time ago :)

The statistics sensor calculates statistical characteristics for a sensor's history. Speaking with a mathematical, statistical, or even physical experimentation mindset, sensor values become dangerous with sparse data. A single sensor reading in the buffer won't provide meaningful values for e.g. min, average or percentile. In fact, this will worsen the situation described by @eljoest "I see spikes and incorrect values from the statistics entities".

The idea behind the minimum samples condition is to ensure a stable statistics result, or unknown otherwise. Scientifically speaking that's exactly how it should be: Provide an average if you can, otherwise be honest that you can't.

I would argue in the opposite direction and claim to increase the threshold for minimum samples needed :)

---

Here is the thing: Statistics can only be calculated correctly, if enough samples are available in the buffer. That is a fact everyone should easily agree with. The addition of keep_last_sample or the reduction of the minimum sample count are just cover-ups of this problem and imho the wrong direction. We need more samples! 😃

I am convinced what the statistics integration is missing is a service, or rather an action as it is called nowadays.

action: statistics.trigger_manual_update
data:
  from_source_entity: true

This action can be called on a schedule or better based on state-triggers and conditions. The user can specifically ensure when and which values go into the statistics sensor to ensure a populated buffer and a meaningful statistical characteristic. A change of the minimum sample count isn't necessary any longer and the use of keep_last_sample is completely avoided. By calling this action in the right manner, the statistics sensor will provide statistical values that actually reflect the nature of your physical source sensor.

Does that make sense? Cheers!

[eljoest]

@ThomDietrich Hi Thom, thanks for the clarification. With the introduction of "max_age" to cope with sensors (or storage implementations) that do not transmit or keep unchanged values, the number of samples is a poor indicator of "valid" calculations. As you mentioned, the calculations would probably need to be changed to account for values just outside the max_age window. As an example consider the following raw data:

time (past)	value
-62	1
-1	100
0	102

With a max_age of 60 the current implementation of average_step would give a value of 101 I believe (it does have 2 values to work with, 100 and 102). My expectation as a user would be a lot closer to 50. If the implementation were to retain (and/or be provided with) and use the last sample just outside the max_age timeframe, the value could be calculated correctly.

On the other hand: Without the updated values at -1 and 0 seconds, the average within the max_age timeframe of 60 should presumably be 1 not "None" as long as the current value of the source is still "1" and not unknown.

This would however require a larger overhaul of the statistics component and its interaction with the data sources. The ability to trigger intermediate calculations would match what I am currently doing: Programatically adding "noise" to the sensor values to force updates at fixed intervals.

[unfug-at-github]

@ThomDietrich: I'm not quite sure I can follow that argument. At the very least, we should leave it to the user to decide whether or not he / she is happy with values calculated based on a small sample set. We are not talking about scientific studies where large sets of samples are a must have here. We could introduce a parameter like "fail if less than x samples in buffer", for cases where large sample sizes are a real requirement. Actually, there already is a counter for the actual number of elements in the buffer that could be used in these situations.

To explain my concrete situation: my inverter is queried every two seconds, this produces updated values every two seconds, so technically speaking there are lots of values to put into a 30 second average for example. The problem is that home assistant is not passing on these values, if they don't change. At night my solar production is 0W. This is measured every two seconds, but what ends up at the statistics sensor is a single 0W measurement performed around sunset. This very accurately presents the current situation, my average production is indeed 0W, so why would I want to see "unknown" instead? By the way, in case there is an outage at night, this would correctly be reported as "unknown", since the updated "unknown" value would make it through the system.

To work around this "unknown" issue you could use a template sensor that either reports the current value or the average from the statistics sensor if it is available. That's what I did in my first attempts. However, that leads to a whole lot of useless templates repeating the same pattern.

Another solution is adding additional dummy-attributes that would force the values to be propagated, or as @eljoest mentioned adding artifical noise.

To be honest, I am slowly getting a little frustrated over this issue. The current sensor is producing results that nobody will be able to understand without looking into the source code. Some of the functions require at least one value, others require at least 2. This is not consistent and can't be explained from a mathematical standpoint. Also, the values that are getting computed don't represent the whole time interval (as explained by @eljoest above), but only the interval between the first and last change within the interval. I have already fixed all these issues here and was hoping to get them added to the statistics component bit by bit.

Anyway, if you think that the current behavior is as it should be, I'll just convert my solution into a separate custom component and leave it at that.

[emontnemery]

@ThomDietrich wrote

The statistics sensor calculates statistical characteristics for a sensor's history. Speaking with a mathematical, statistical, or even physical experimentation mindset, sensor values become dangerous with sparse data. A single sensor reading in the buffer won't provide meaningful values for e.g. min, average or percentile

I think that is an issue both if there is a single or two observations? I'd suggest to not block calculating characteristics unless they are not defined.

[emontnemery]

@unfug-at-github you mention that

To explain my concrete situation: my inverter is queried every two seconds, this produces updated values every two seconds, so technically speaking there are lots of values to put into a 30 second average for example. The problem is that home assistant is not passing on these values, if they don't change.

The solution to this is the newly introduced async_track_state_report_event which listens to state reports which fire every time a source sensor state is set instead of state changes which only fire when the state changes. Improving the statistics sensor was a major reason for introducing that event, but the functionality has not yet been added.

A PR which changes the statistics sensor to consume state reports would be very welcome.

For reference, here's a PR for the integration sensor which does that: #113869. Note that there were a couple of follow-up PRs, the initial implemented directly subscribed to the events which performed very poorly.

A different approach is to periodically sample the input sensor instead. I don't remember if the statistics sensor already has that functionality? Here's a PR for the integration sensor which added that: #110685

[unfug-at-github]

@emontnemery: Thanks for your explanations. I agree that getting more frequent updates would take many of the pain points away or at least mellow them down significantly:

• there would always be many values which would go in the direction of what @ThomDietrich envisions
• the intervals before and after the last value would usually be rather small, so the computed values would be closer to what people would expect to see, the errors might be small enough to ignore
• the values would be updated more frequently, so the forced refresh I implemented would not really be necessary
• the spikes caused by the race conditions will not be fixed, but we already discussed an alternative for that

Having said that, I still think that the changes proposed in this PR make sense. Why average_step and average_linear need two values at least, while average_timeless and mean just need one, can't be justified in my eyes.

Implementing the changes as you proposed them makes sense in my eyes. However, it will generate quite a bit of effort

• as you mentioned there might be performance issues (maybe they can be prevented by copying with pride from Riemann)
• probably new tests are going to be expected
• some tests may fail because repeating values will then be passed to the statistics functions as multiple values instead of as a single value

I haven't decided yet whether I'm going to give that a try, after all I have a solution that works for me now. I must admit that I have underestimated the amount of effort that goes into contributing even comparably small changes here.

[ThomDietrich]

Hey friends,

@emontnemery

I think that is an issue both if there is a single or two observations? I'd suggest to not block calculating characteristics unless they are not defined.

I have no strong preference on this detail. Just a few samples is shit either way. Instead of dealing with low sample count we need to help the user to keep a high sample count. Therefore ...

@emontnemery

The solution to this is the newly introduced async_track_state_report_event which listens to state reports...

Erik, I completely forgot about this topic. We did discuss it before. I totally agree, this would be the most important improvement to the statistics component today.

@emontnemery

A different approach is to periodically sample the input sensor instead.

This is an additional improvement and exactly what the suggestion of the statistics.trigger_manual_update action described above is supposed to accomplish. A build-in periodic sampling doesn't cut it, as frequency and other details can't be generalized. The action provides needed control to the user.
That said, of course a convenience configuration of the satatistics component that schedules a periodic call of that action would be a nice to have for ease of use...

@unfug-at-github

I haven't decided yet whether I'm going to give that a try, after all I have a solution that works for me now. I must admit that I have underestimated the amount of effort that goes into contributing even comparably small changes here.

I am sorry to hear this. Being honest, that is the same reason why I haven't been active so long. Between family and building a house, I did not have the time and energy to deal with the rigorous review process here.

@unfug-at-github

However, it will generate quite a bit of effort

I agree, on the other hand, I am convinced my proposed action would solve your and many more issues and serve many use cases. If you can find the motivation to implement it, I promise to provide you with a swift and quick review&approval 😺

[emontnemery]

Based on the discussion, I agree we can go ahead and merge this PR.

However, before merge:

• The PR description needs a breaking change section to alert users to the changed behavior. It's enough to list the characteristics which previously needed 2 samples to produce output but now only need a single one.
• Add or adjust test cases as needed to ensure full test coverage of the new behavior

PS: Thanks for your patience so far @unfug-at-github 👍

[unfug-at-github]

@emontnemery: We are getting a little of-topic here, but are you sure these state report events ("state updated, but not changed") would travel through cascaded layers of template sensors? Otherwise, listening for them will only be a partial improvement, that will not work for values computed by the template sensors.

[emontnemery]

@unfug-at-github All sensors which consume states from one or several input sensors need to be updated to recalculate on state reports instead of on state changes. Template sensors will need some extra consideration because state_reported events are not exposed to the template engine due to the potentially detrimental performance impact. I don't have a good answer for how this can be achieved.

You're most welcome to contact me on Discord, my nickname there is @emontnemery, if you want to discuss this.

[emontnemery]

Thanks @unfug-at-github 👍

Congrats on the first accepted PR, we hope to see more of them! 🎉