On Wed, Apr 20, 2016 at 5:38 AM, Stephen Canon <[email protected]> wrote: > Hi Xiaodi — > > Thanks for the detailed comments. Some thoughts inline. > >> On Apr 19, 2016, at 6:34 PM, Xiaodi Wu via swift-evolution >> <[email protected]> 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 > _______________________________________________ swift-evolution mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-evolution
