Comments inline. > On Apr 20, 2016, at 12:04 PM, Xiaodi Wu <[email protected]> wrote: > > 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.
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). >>> 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 [email protected] https://lists.swift.org/mailman/listinfo/swift-evolution
