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

[Stephen Canon]

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.

> 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.

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`.

`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”?

> 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).  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 %.]

– Steve