[swift-evolution] [Review] SE-0067: Enhanced Floating Point Protocols

[Xiaodi Wu]

On Wed, Apr 20, 2016 at 5:38 AM, Stephen Canon <scanon at apple.com> wrote:
> Hi Xiaodi —
>
> Thanks for the detailed comments.  Some thoughts inline.
>
>> On Apr 19, 2016, at 6:34 PM, Xiaodi Wu via swift-evolution <swift-evolution at swift.org> wrote:
>>
>> * What is your evaluation of the proposal?
>>
>> +1 in intent. Specifics require further refinement. For example:
>>
>> Internal inconsistencies in capitalization:
>> * `signalingNaN` but `isSignalingNan` and `isNan`
>
> This is a typo.  Should be signalingNan.
>
>> Parameter labels, or whatever they're called now, do not reflect newly
>> adopted Swift syntax in SE-0046:
>> * `static func maximum(x: Self, _ y: Self) -> Self` should be `static
>> func maximum(_ x: Self, _ y: Self) -> Self`, etc.
>>
>> Infelicitous use of prepositions to conform superficially to new
>> naming guidelines:
>> * `isEqual(to:)` is fine, but for consistency there's
>> `isLessThanOrEqual(to:)`, which is not fine, because the preposition
>> "to" applies only to "equal" and not to "less than”
>>
>> Since `adding(_:)` is instead now an operator in the current version
>> of the proposal, could comparison functions also be operators only?
>
> They could, but you still need isUnordered(with: ) and isTotallyOrdered(with: ), as they don’t have operator equivalents.

My two cents are that the comparison functions that have operators
should stick with those operators only instead of the names.
Stylistically, I think `isLessThanOrEqual(to:)` is less than
cromulent.

Some thoughts on `isUnordered(with:)` and `isTotallyOrdered(with:)`--

1. As you mention below, implementations are free to expose
IEEE754-mandated operations with names that differ from the
specification. Looking to Java and C# as examples of how other C-style
languages implement floating point types, C-style languages have taken
a free hand in how they deal with comparison with NaN.

So long as we're not going down the road of having methods that expose
distinct unordered-quiet and unordered-signaling comparison predicates
as defined in IEEE754 (do any C-style languages do that?), there's
probably some freedom to determine whether `isUnordered(with:)` is
necessary as a standalone method or whether it's already sufficiently
provided for by other methods. In the end, `x.isUnordered(with: y)` is
really equivalent to `x.isNan || y.isNan`, which can be immediately
grokked by any reader of code.

2. `isTotallyOrdered(with:)` isn't quite apt as a name. As I
understand it, totalOrder(x, y) "reflects" a total ordering such that,
when x is ordered "below" y, the return value is true. (Both quoted
words reflect the terminology used in the specification.) So perhaps
`isTotallyOrdered(below:)` or even just `isOrdered(below:)` would be
more apt. Certainly `with` is not the correct label for the parameter,
as any two values that don't compare unordered should be "totally
ordered with" each other. This is especially problematic if we're
going to keep `isUnordered(with:)`, since those two methods would
suggest a complementary logical relationship that just isn't there.


>
>> Incorrect nomenclature in an attempt to correct previously misleading
>> nomenclature:
>> * `leastMagnitude` should be `leastPositive` or `minPositive`, because
>> magnitudes can be zero: it's bonkers that
>> `Double.minimumMagnitude(0.0, Double.leastMagnitude) <
>> Double.leastMagnitude`!
>> (Likewise, `leastNormalMagnitude` should be `leastNormalPositive` or
>> `minPositive`, and `greatestFiniteMagnitude` should be
>> `greatestFinite` or `maxFinite`)
>>
>> Inconsistencies with Integer protocols (at least, as they are currently):
>> * properties named "least..." or "greatest..." are inconsistent with
>> conceptually similar properties such as `Int.min` and `Int.max`
>
> `min` and `max` were deliberately avoided based on a discussion with the Apple standard library team; these properties don’t really behave like the integer bounds properties, so naming them similarly may be confusing.

I agree absolutely that `min` and `max`, standalone, are misleading
and shouldn't be used. That said, I would argue that `maxFinite` is
demonstrably the correct name because it's the most succinct one
that's both accurate and consistent with other usage. Adding
"magnitude" clarifies nothing here and in fact made me do a
double-take on first reading: I had to think for a split second
whether there exists a greater floating point value that's a finite
non-magnitude: it's of course absurd, but you have to think about it
the first time. Meanwhile, `greatestFinite` means exactly the same
thing as `maxFinite`, but now you're introducing a different word
where it's not necessary to clarify the meaning or differentiate from
standalone `max`.

>
> Your point about magnitudes being non-zero is reasonable, but I think you’ve taken it a step to far; it could be corrected by simply changing `leastMagnitude` to either `leastPositiveMagnitude` or `leastNonzeroMagnitude`.
>

By the same reasoning here, `minPositive` (instead of
`leastMagnitude`) is equally accurate, more consistent with usage
elsewhere, and probably more accessible to non-native English
speakers. Throughout the IEEE specification, "magnitude" is used in
relation to absolute values, which is not really in play here.

In any case, we agree that `leastMagnitude` must at minimum be renamed
to exclude zero.

> `leastNormalMagnitude` and `greatestFiniteMagnitude` are accurate as is.  (`minPositive`, on the other hand, would be exceedingly misleading).  Can you expand on why you want to change them?  It seems like you simply prefer “positive” to “magnitude”?

I made a typo here (I blame jetlag): `minPositiveNormal` was what I
meant to type. Again, my rationale is that it is the least deviation
from `min` that accurately describes what's going on using the
simplest words. Here, the distinction between normal and subnormal is
unavoidable, but we don't need to talk about magnitudes, and there
really is no difference between "min" and "least".

Also potentially useful (actually, definitely useful in implementing
floating point strides) would be the properties `maxExactInteger`
(and, I suppose, a corresponding `minExactInteger`).


>
>> Use of one term-of-art (`ulp`), as Swift naming guidelines allow and
>> even encourage, but failure to use more widely understood terms of
>> art:
>> * `squareRoot()` should be `sqrt()`
>> * something really ought to be done about
>> `truncatingRemainder(dividingBy:)`--the fact that the comments tell
>> the reader that `truncatingRemainder(dividingBy:)` is equivalent to C
>> `fmod` suggests to me that `fmod` may be a widely understood
>> term-of-art
>> I argue strongly that Swift's naming guidelines about terms-of-art
>> should be understood to encourage terms used widely in other languages
>> for basic mathematical functions instead of written-out English
>> equivalents--e.g. `asinh` instead of`inverseHyperbolicSine`. Looking
>> to other C-style languages, all seem to accept these shorter terms
>> as-is without writing them out.
>
> sqrt( ) I could support, but fmod( ) is an absolutely terrible name for a relatively rarely-used function.  If there were a good term-of-art, I would want to use it, but AFAIK there isn’t.
>
> I should note that the free functions sqrt( ) and fmod( ) won’t go away with this proposal.  They will continue to be supplied by the math overlay for Float, Double, CGFloat, just not for the FloatingPoint protocol.  So why do we need them in the FloatingPoint protocol at all?
>
> The squareRoot( ) and remainder( ) methods are distinct from most of the other <math.h> functions in that IEEE 754 considers them to be "basic operations” as defined by clause 5 of the standard (IEEE 754 spells out the name “squareRoot” FWIW, though there’s no requirement that we follow that).

Even IEEE754 is interestingly inconsistent here. Throughout, it's
written as "squareRoot", but when it comes to "recommended" functions,
the reciprocal of the square root is written "rSqrt" (see table 9.1).
I'd highly recommend setting a good example of when things are
terms-of-art by going with "sqrt".

While we're on the topic of IEEE754 section 5 functions, we're missing
abs(x) in the protocol.

And speaking of absolute value functions, IEEE754 calls it "minNumMag"
and "maxNumMag" when comparing two values but, when it comes to
summing, recommends a function called "sumAbs". A missed opportunity
for consistency, IMO. When implementing in Swift, then, was there a
rationale for having `minimumMagnitude` instead of the much shorter
but equally grokkable `minAbs` (abs being, IMO, a term-of-art like
sqrt)?

And then speaking of implementations of IEEE minNum and maxNum--are
those static methods necessary in the protocol? What is gained over
having just the comparison operators?


> Because of this it makes sense to require them as methods in the FloatingPoint protocol, rather than only as free functions.  [truncatingRemainder( ) is not required by IEEE 754, but it doesn’t impose a significant implementation burden and eases the transition for folks currently using operator %.]

I'm not sure I buy the "ease" argument. Surely, if folks are using
operator %, it's equivalently difficult to transition either to the
free function or to `truncatingRemainder()`? Suppose you deprecate %
and recommend fmod(), then just leave `truncatingRemainder()` out of
the protocol. After all, the rationale for deprecation is that people
aren't using it correctly...

>
> – Steve
>