docs: Fix specification clarity of smoothstep

[lgritz]

Swap the confusing edge0/edge1 nomenclature of the smoothstep
declaration to the more intuitive low/high.

It was pointed out on Slack by Arnon Marcus that the spec's
description of smoothstep was ambiguous about the behavior when
low==high: does it return 0 or 1 if x is the same value? The
documentation is unclear.

The implementation returned 1, which I think is the "correct" behavior
in the sense that it matches the results of step() function with that
edge. So update the documentation to match.

Also Arnon pointed out that things are especially weird if low > high,
it's non-monotonic. This seems to be fixed by simply reversing the
relative order of the if x < low and if x >= high tests:
basically, it also makes it match step(x, high) and be monotonic.

This is a cleaner formal definition of what smoothstep should do,
namely:

if (x >= high) {
    return 1.0f;
} else if (x < low) {
    return 0.0f;
} else {
    float t = (x - low) / (high - low);
    return (3.0f-2.0f*t)*(t*t);
}

[HardCoreCodin]

There would still be ambiguity when edge1 < x < edge0 or when edge1 = x < edge0.
So the distinction of < vs. <= is not the only source of ambiguity.
It is regarding which of the 2 comparisons is done first in code - regardless of < vs. <=.
For example, even with changing the spec to use < instead of <= for the low case, when edge1 < x < edge0 (i.e: edge1=2, x=3, edge0=4) or when edge1 = x < edge0 (i.e: edge1=3, x=3, edge0=4):
This implementation would return 0:

    if (x < e0) return 0.0f;
    else if (x >= e1) return 1.0f;
    else {
        float t = (x - e0)/(e1 - e0);
        return (3.0f-2.0f*t)*(t*t);
    }

This implementation would return 1:

    if (x >= e1) return 1.0f;
    else if (x < e0) return 0.0f;
    else {
        float t = (x - e0)/(e1 - e0);
        return (3.0f-2.0f*t)*(t*t);
    }

Both implementations comply with the same updated specification as proposed, and yet - they would return different results.
A better phrasing might be to incorporate ... . Otherwise, if ... somewhere in the spec, thereby implying an ordering to the checks.

[lgritz]

It was clearly the case that the prior < low vs <= low generated uncertainty as to the actual return value when val==low==high (is it 0 or is it 1?).

Oh, I see, you're talking about cases where they intentionally pass high < low?

[lgritz]

Would it help if the spec explicitly spelled out the code equivalent to show the order of comparisons?

    if (x < low) return 0.0f;
    else if (x >= high) return 1.0f;
    else {
        float t = (x - low)/(high - low);
        return (3.0f-2.0f*t)*(t*t);
    }

I think the desired behavior when low==high, matching step() that has only one threshold value, was the clear choice.

I admit that I haven't thought as carefully about what is the best behavior when somebody passes the nonsensical high < low. Is the above code adequate? I'm thinking maybe not. How about switching the order of comparisons:

    if (x >= high) return 1.0f;
    else if (x < low) return 0.0f;
    else {
        float t = (x - low)/(high - low);
        return (3.0f-2.0f*t)*(t*t);
    }

I think that makes it monotonic no matter what the relative order of low and high, right? We can say in this case that if high < low, it also falls back to matching step() with a threshold of high (which in this case is the lower of the two values).

An alternative specification would be to implicitly swap low and high if low > high. In other words, you just have two thresholds, specified in either order, and it's 0 when lower than the lowest threshold, and interpolates to 1 as it goes from the lower to the higher edge threshold. I like the elegance of that, but I don't see how to do it without adding expense, since it would require an additional conditional to detect the low>high case.

[HardCoreCodin]

Swapping would not only make it more expensive, it would also make it inconsistent - the "non sensical" case(s) occur naturally and implicitly when using image texture maps (even very simple ones). You could make an argument that this function is not aimed to be used with images, but that would not be a very strong argument.

I am not sure there necessarily even is a right or wrong ordering (I haven't thought hard on that and have no stake in it), what matters is that the specification states whichever is chosen to be the desired behavior, as opposed to leaving it to interpretation or arbitrary implementation choice.

[lgritz]

Yeah, I think the simplest solution is just to test >= high first. I believe that leads to consistent behavior, and also it's easy to explain that smoothstep(low,high) == step(high) any time that high <= low.

[lgritz]

Updated:

Swapped the relative order of the >=high and <low tests in order to make the high<low case also be sane.

[lgritz]

Comments?

[lgritz]

Last chance for objections...

[sfriedmapixar]

Aside from a typo in stdlib.md, LGTM

[sfriedmapixar]

Shouldn't this be x $\le$ low to match the .tex

[HardCoreCodin]

I still feel the need to point out that any phasing of the form " returns 0 if x something low, and 1 if x something high" is going to remain ambiguous, because these are 2 separate checks against 2 separate inputs(!) which necessarily makes it possible for both conditions to be true - regardless of what comparison is used - at which point the phrasing is not specifying what should happen (which of the 2 satisfied conditions should determine the output).