oliexdev / openScale

Open-source weight and body metrics tracker, with support for Bluetooth scales
GNU General Public License v3.0
1.68k stars 292 forks source link

Simple moving average on the chart #968

Closed undefiened closed 1 year ago

undefiened commented 1 year ago

This code adds a simple moving average trend line with an option to select the time period over which the line should be averaged.

It is different from the currently implemented exponential smoothing because the simple moving average does not account for the old measurements: even though the old measurements are supposed to contribute little, they still contribute, and therefore the current state of things isn't represented properly (at least for me).

There are no future predictions in this implementation, only the moving average.

Also, my simple moving average should work properly with irregular measurements, while the current implementation of the exponential smoothing works incorrectly when there are long periods without data since it works measurement-wise, not time-wise. I.e., if I have the following list of measurements (an arbitrary example with random numbers)

January 1st 2022 - 100 kg December 1st 2023 - 50 kg December 2nd 2023 - 50 kg ... December 29th 2023 - 50 kg

then the whole thing would be skewed by the January 1st 2022 measurement, even though technically it was more than a year ago compared to the December 1st 2023 measurement.

I tried locally implementing interpolation to fix exponential smoothing when there are gaps in data by filling all missing days with simple linear interpolation, it works better that way, but still looks different from the simple moving average. (It works slowly, so I decided to not put my attempt at the interpolation fix into this pull request)

Also, this code should somewhat address https://github.com/oliexdev/openScale/issues/716 although not directly.

I tried this code on my data and it looks fine to me.

Please let me know if something doesn't look good or needs to be done, I am not familiar with the rules on how one should contribute to this repository.

oliexdev commented 1 year ago

Takes a little bit more to review it but somehow we have now double lines of each measurements!? Was that intended? (One normal and one average line but I see no difference)

undefiened commented 1 year ago

That is weird, I am not entirely sure what you mean. Probably I messed up something.

This is how it looks to me.

Without moving average: 2023-07-22_15-22_1

With moving average: 2023-07-22_15-22

oliexdev commented 1 year ago

please go the chart fragment and enable the legend.

undefiened commented 1 year ago

please go the chart fragment and enable the legend.

Got it, you mean that it is shown two times in the legend. This is the same behavior as for the trendline, so not sure how I should deal with it.

undefiened commented 1 year ago

I removed the entry for the moving average from the legend. Would that be a good solution?

oliexdev commented 1 year ago

My fault no please add the legend again. What's only now confusing is the naming of "Trendline" and "Simple moving average" in the options / code now. :/ Nobody knows what method is behind "Trendline" (problem not new) and "Simple Moving Average" could be also said as a "Trendline". Maybe its better to add in the graph settings an option to enable/disable the trendline in general and add a second option to select the method of calculation!? (none, moving average, exponentially smoothed moving average and future ones). Would also solve the confusion with many parallel visible trendlines. But would be more complex and a lot of renaming have to be done. What do you think? Do you have a better idea?

undefiened commented 1 year ago

You are right. I did the changes in the settings and the code, does it look better now? (I will fix the merge conflict later, it shouldn't be a big issue)

undefiened commented 1 year ago

@oliexdev thank you again for the quick reviews!

oliexdev commented 1 year ago

Really big thanks for your awesome implementation @undefiened :fireworks:

undefiened commented 1 year ago

Thank you very much for all the help and patience and especially for this app in the first place :)