Skip to content

Fix double-mean bug in climatology when detrend='none'#23

Open
loganchal wants to merge 1 commit intochadagreene:masterfrom
loganchal:patch-1
Open

Fix double-mean bug in climatology when detrend='none'#23
loganchal wants to merge 1 commit intochadagreene:masterfrom
loganchal:patch-1

Conversation

@loganchal
Copy link
Copy Markdown

When running climatology with ('detrend','none'), the mean of the data is added to the output twice. The 'none' option skips the initial detrending step, so the mean remains inside Ac. Then meanAr is added back unconditionally at the end, doubling it.

Fix: extend the conditional to also exclude the 'none' case.

On the buggy code this reproducer will return all 20s for a string of constant 10s before the fix:

t = datenum(2000,1,1):datenum(2000,12,31);
data = ones(size(t)) * 10;
clim = climatology(data, t, 'daily', 'detrend', 'none');
mean(clim)  % returns 20 instead of 10 before fix

When running climatology with ('detrend','none'), the mean of the data is added to the output twice. The 'none' option skips the initial detrending step, so the mean remains inside Ac. Then meanAr is added back unconditionally at the end, doubling it.

Fix: extend the conditional to also exclude the 'none' case.

On the buggy code this reproducer will return all 20s for a string of constant 10s before the fix:

    t = datenum(2000,1,1):datenum(2000,12,31);
    data = ones(size(t)) * 10;
    clim = climatology(data, t, 'daily', 'detrend', 'none');
    mean(clim)  % returns 20 instead of 10 before fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant