Comments inline.
Hi Xiaodi —
Thanks for the detailed comments. Some thoughts inline.
* 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).
···
On Apr 20, 2016, at 12:04 PM, Xiaodi Wu <xiaodi.wu@gmail.com> wrote:
On Wed, Apr 20, 2016 at 5:38 AM, Stephen Canon <scanon@apple.com> wrote:
On Apr 19, 2016, at 6:34 PM, Xiaodi Wu via swift-evolution <swift-evolution@swift.org> wrote: