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

Wed Apr 20 15:07:04 CDT 2016

on Wed Apr 20 2016, Stephen Canon <swift-evolution at swift.org> wrote:

> Comments inline.
>
>> On Apr 20, 2016, at 12:04 PM, Xiaodi Wu <xiaodi.wu at gmail.com> wrote:
>> 
>> 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.
>
> Yes, if we drop isUnordered(with:), then renaming this isOrdered(below:) would work.  I’m OK with this approach.
>
>>>> 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`.
>
> My biggest concern with `maxFinite` is that it would seem to also
> require a `minFinite`, which would then be easily confused with
> e.g. DBL_MIN (which means something radically different), and also
> implies that they might not be symmetric after all.  To my ear,
> “magnitude” more readily suggests the existing symmetry: namely that I
> can get the finite bounds as +/-.greatestFiniteMagnitude.
>
> FWIW, 754 actually explicitly describes least[Positive]Magnitude as
> “the positive number of least magnitude” (the definition of nextUp,
> 5.3.1).
>
>>> 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”.
>
> One is a word, the other is an abbreviation.  The swift guidelines,
> for better or worse, counsel us to “avoid abbreviations”.  The fact
> that “min" is a term of art makes it plausible, but I don’t think
> we’re desperate to save two characters in the name.
>
>> Also potentially useful (actually, definitely useful in implementing
>> floating point strides) would be the properties `maxExactInteger`
>> (and, I suppose, a corresponding `minExactInteger`).
>
> An early draft of the protocol had these.  Naming this property is
> *hard*, because every floating-point value larger than
> `maxExactInteger` is … an exact integer.  If you want to be
> unambiguously precise, you end up with something horrible.  Ultimately
> I punted on this issue, but I would definitely support adding it in
> the future if an appropriate name can be found, or if a compelling use
> case arises (I don’t think it’s actually needed for implementing
> strides).

Isn't this really maxResultOfAdding1?

>
>
>>>> 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”.
>
> Clause 9, being non-normative, didn't receive nearly as much editorial attention.
>
>> While we're on the topic of IEEE754 section 5 functions, we're missing
>> abs(x) in the protocol.
>
> abs() would have come from SignedArithmetic, but you’re right that it’s now missing and should be added to FloatingPoint.
>
>> 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)?
>
> maxAbs is unclear (to me); I could easily imagine that it returns
> max(abs(x), abs(y)).  Given that these methods have no precedent in
> C-family languages, there isn’t an actual term of art to follow, and
> the meaning of maxAbs isn’t particularly obvious.
>
>> 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?
>
> While one can write these operations in terms of the comparison
> operators, it’s a bit unwieldy to do so, and requires some heroics
> from the optimizer to turn such an expanded definition into e.g. the
> arm64 FMAXNM instruction or the AVX-512 VRANGE instruction.  It’s
> somewhat easier to wire up those optimizations this way.
>
> Similarly, having these methods directly available is a useful
> optimization hook for user-defined types that conform to the protocol
> (though the protocol *will* provide default implementations, so you’re
> not obligated to do so).
>
>>> 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...
>
> fmod() wraps the C stdlib fmod(), so only exists for types supported
> by the C stdlib.  The truncatingRemainder() method is available for
> all FloatingPoint types (these two sets of types are equivalent for
> Swift today, but if someone wants to write a library type that
> conforms to FloatingPoint, it would provide truncatingRemainder).
>
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution

-- 
Dave