daisukelab
commented
2 years ago Hi @WhiteTeaDragon, thank you for your interest and for pointing this out.
I think you are correct.
I plan to fix this in near future, then let me ask you to wait for a fix for now.
I have not tested yet, but the fix would be like this. As you mentioned, we can make the incrementing n should be before averaging.
class RunningMean:
"""Running mean calculator for arbitrary axis configuration."""
def __init__(self, axis):
self.n = 0
self.axis = axis
def put(self, x):
# https://math.stackexchange.com/questions/106700/incremental-averageing
self.n += 1
if self.n == 1:
self.mu = x.mean(self.axis, keepdims=True)
else:
self.mu += (x.mean(self.axis, keepdims=True) - self.mu) / self.n
def __call__(self):
return self.mu
def __len__(self):
return self.n
WhiteTeaDragon
Thank you for the fascinating paper and the code to reproduce it!
I think there might be a problem in RunningMean. The current formula (the same in v1 and v2) looks like this:
$$ mn = m{n - 1} + \frac{an - m{n - 1}}{n - 1}, $$
which is inconsistent with the correct formula listed on StackOverflow:
$$ mn = m{n - 1} + \frac{an - m{n - 1}}{n}. $$
The problem is that self.n is incremented after the new mean is computed. Could you please either correct me if I am wrong or correct the code?