stale_values_diff and interpolation_diff : option to label all values

[cwhanse]

Consider adding an option to label all values in a stale / interpolation window as stale, rather than only labelling the end value in a window.

[wfvining]

I proposed a kwarg include_prefix to do this in #37. Not sure about the name, but that PR includes the following snippet which will be useful for this:

    flags = endpoints
    while window > 0:
        window = window - 1
        flags = flags | endpoints.shift(-window).fillna(False)

where endpoints is the Boolean series with the ends of the windows labelled. Not sure if this is the most efficient solution, but window is typically going to be much much smaller than the entire series, so the loop wont have to execute too many times.

We can pull this out as a function that is applied if include_prefix is True.

[wfvining]

label_all seems like a better name than include_prefix

[wholmgren]

Do you have a particular use case in mind for this? Is the idea that if the stale/interpolation detector triggers, then it's most likely that all points (or at least points[1:]) in the window are bad? That seems reasonable, but I see that #37 #46 left the original behavior as the default.

Ultimately I'm wondering how valuable it would be to incorporate into the Arbiter where we can't expose all of the options to the web end user.

[wfvining]

@wholmgren that's the idea. #37 still needs work (and rebasing), I wanted to get this option added separately from that PR.

If you have many short periods of interpolation (short meaning close to the same length as the window size) then marking all the values in the window is going to be very beneficial. If the periods of interpolation/staleness are very long relative to the window length then there is probably limited benefit. You and @cwhanse would know much better than me which of these two cases is most common.

[wfvining]

I'm reopening so we can discuss which behavior should be the default. We should make sure the default is consistent between the functions ported from SFA and the function added in #37

[wholmgren]

I don't think you need to worry about keeping the default behavior consistent between the libraries. I hope SFA eventually wraps pvanalytics but that it's just one application that's using the library. In this case I'm only trying to understand the motivation so I can decide what the Arbiter should present to end users.

[wfvining]

@wholmgren Do you have an opinion about the default behavior? I genuinely do not know which way is "better," but I am leaning towards making label_all=True by default.

[cwhanse]

I think label_all=True is more intuitive. The algorithm went into SFA with labeling the last point because I couldn't figure out an efficient way to label all the points in a window.

[wfvining]

I went to make this change and I realized that the way label_all is implemented the very first point is included as well. This may not actually be the behavior we want since it is likely that the very first point in a stale/interpolated window is actually okay.

Do we want to make this more complicated and change from a boolean to a string:

• label='all' meaning all points in the window including points[0],
• label='tail' meaning all points in the window except points[0],
• label='end' meaning the current default behavior (endpoints only).

[wholmgren]

label='tail' meaning all points in the window except points[0],

I like this as the default.

I'm not sure about the 'tail'/'end' names but I don't have better ones to offer.