on Wed Apr 20 2016, Stephen Canon <[email protected]> wrote:
> 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). 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 > [email protected] > https://lists.swift.org/mailman/listinfo/swift-evolution -- Dave _______________________________________________ swift-evolution mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-evolution
