float32 and UoM assertion functions

[jwosty]

I've added float32 versions of all the float assertion functions. I've also extended both the float and float32 versions of all of those to support floats/float32s with units of measure.

List of functions added:

• floatClosef
• floatLessThanOrClosef
• floatGreaterThanOrClosef
• isNotNaNf
• isNotPositiveInfinityf
• isNotNegativeInfinityf
• isNotInfinityf

For example we can now write:

Expect.floatClose Accuracy.low<m> 1.0<m> 2.0<m> "result"

and:

Expect.floatClosef Accuracy.lowf<m> 1.0f<m> 2.0f<m> "result"

[farlee2121]

A few thoughts here.

32-bit variant naming

I'm tracking with the float32 methods. I had to build some context before I understood the naming scheme though.
Using an f suffix seems a bit ambiguous. Both method groups are for floats, the difference is the precision.

How do you feel about a naming scheme like float32Close?
The NaN/infinity methods are a bit less direct. Maybe isNotNaN32 and isNotPositiveInfinity32?
The accuracy types probably follow whatever we decide for NaN/infinity (Accuracy32?)

Would it be more clear to split the accuracy constants into two modules? So instead of having Accuracy.low and Accuracy.lowf we'd have Accuracy.low and Accuracy32.low

Units of measure

If I understand right, the motivation here is to operate on values that have units without the caller needing to unwrap those units?

It feels like it adds some ceremony to the general case that I'm uncomfortable with.

Maybe I'm not understanding fully. I'd like to hear more of what you had in mind.

[jwosty]

Using an f suffix seems a bit ambiguous.

It matches precedent set elsewhere. For example: the BCL MathF class and the fact that both F# and C# use f to indicate a float32 literal (i.e. 1.0f). I'm pretty sure there are more examples; I just can't think of them right now.

How do you feel about a naming scheme like float32Close?

It's certainly obvious what it is, but so wordy IMO. Which might disincentivize one too strongly from using float32. Granted, it's better than the existing alternative (manually casting everything to float32), but only a little.

The NaN/infinity methods are a bit less direct. Maybe isNotNaN32 and isNotPositiveInfinity32?

I could see that. For some reason I don't really like it stylistically, but that's super subjective and not a very good actual argument, haha. So I could live with that :)

The accuracy types probably follow whatever we decide for NaN/infinity (Accuracy32?)

Agreed.

Would it be more clear to split the accuracy constants into two modules? So instead of having Accuracy.low and Accuracy.lowf we'd have Accuracy.low and Accuracy32.low

I'd have no issue with that; makes perfect sense. Though if it were up to me I'd probably name it AccuracyF -- the same points from above apply here.

---

There's actually another option we haven't discussed: using SRTP to overload all the existing functions to work with both floats and float32s.

Pros:

• Completely sidesteps the naming debate :)
• No need for the user to even really be aware of the differences; it "just works"

Cons:

• Slightly increases maintenance burden (SRTP's are someone arcane)
• Error messages are not the best when types mismatch (like method overloads)

Though the first downside is somewhat mitigated by the fact that this wouldn't be a super complex use of SRTP's (we're not using it simulating monads or anything like FSharpPlus does for example). Just simple function overloading for floats and float32s.

---

If I understand right, the motivation here is to operate on values that have units without the caller needing to unwrap those units?
It feels like it adds some ceremony to the general case that I'm uncomfortable with.
Maybe I'm not understanding fully. I'd like to hear more of what you had in mind.

Yes it's to make it easier to assert against values with units-of-measure.

It doesn't add any burden to the user of the library. Just the function definitions themselves. I basically just made it all generic with respect to UoM, as F# by default infers unitless (non-generic) by default.

If you have a codebase that uses a lot of UoM, it gets pretty annoying to write tests for it. For example, given this code:

type Point<[<Measure>] 'u> = { X: float<'u>; Y: float<'u> }
module Point =
    let length (p: Point<'u>) : float<'u> = sqrt ((p.X * p.X) + (p.Y * p.Y))
    let create (x, y) = { X = x; Y = y }

We have to write

open Expecto.Expect
open Expecto.Flip
open FSharp.Data.UnitSystems.SI.UnitSymbols

[<Tests>]
let tests =
    testCase "length" (fun () ->
        (Point.length (Point.create (3.0<m>, 4.0<m>)))
        |> float
        |> Expect.floatClose "result" Accuracy.high 5.0<m>
    )

When we'd really just like to write:

open Expecto.Expect
open Expecto.Flip
open FSharp.Data.UnitSystems.SI.UnitSymbols

[<Tests>]
let tests =
    testCase "length" (fun () ->
        // no cast needed
        (Point.length (Point.create (3.0<m>, 4.0<m>)))
        |> Expect.floatClose "result" Accuracy.high 5.0<m> 
    )

Granted, this is a particularly simple case on its own, but when your code and tests start to get a little more complex, it gets more and more annoying. Also - this is an issue with all test frameworks anyway; it's just that Expecto here has a chance to actually do something about it since it is F#-aware and set itself apart in this area.

And to reiterate - it's strictly backwards compatible. No changes needed for users who doesn't employ units of measure.

[jwosty]

One more thing: I just realized another approach to Accuracy would be to define both the float and float32 values in one type:

// NOTE: I've written what it would look like with UoM enabled -- this could still be done without that bit (just drop all the units)
type Accuracy<[<Measure>] 'u> = 
    { absolute: float; relative: float<'u>
      absolutef: float32; relative: float32<'u> }
module Accuracy =
    let low<[<Measure>] 'u> = // ...
    let high<[<Measure>] 'u> = // ...
    // etc

Then you just use the same single Accuracy type and module everywhere.

[farlee2121]

Naming

Seems like we should get some more opinions.

@ratsclub @haf @rynoV @Numpsy
Do any of you have feelings about the naming convention for float vs float32 assertions?

I.e. floatClose exists, so would the float32 version be called floatClosef, floatClose32, float32Close, other?
Or, alternatively, should we use statically resolved type parameters to support both types with the same assertion name?

Statically Resolved Type Parameters

I often wish for overloads in this kind of situation. But, I feel like overloads are non-standard in F#.
Separate methods seems the more idiomatic path.

Units of Measure

I guess I didn't look close enough. I'm onboard now.

Doubled-up accuracy definitions

I prefer separate accuracy types. Grouping them in one type might consolidate our standard accuracy levels, but it creates incidental coupling. It assumes that users will only ever define accuracy levels that apply to both float and float32.

[Numpsy]

I'm not sure about the 'f' vs '32' naming - i see the point about f being used for literals and some other math things, but then there are also a bunch of functions that use an f suffix to infer they do formatting (failtestf and similar) so maybe avoiding that and using 'float32' is better? I don't have any really strong opinion though.

[haf]

Imo:
Float should link to what the CLR / F# considers float. It’s 64 bits in f# I believe. Float32 is that specifically. So three families with no suffix, two with.

[jwosty]

I often wish for overloads in this kind of situation. But, I feel like overloads are non-standard in F#.
Separate methods seems the more idiomatic path.

The F# team discourages their widespread careless use because they're kind of ugly. That's not to say that you shouldn't use them, just that you should be really sparing and careful with them. There are legitimate uses. Libraries are probably one of the places where it can really make sense. Whether this is a legit use or and abuse is up for debate :)

In fact one advantage of doing so is that we could, for example, define a super generic close function using SRTP which is extensible by the user of the library to their own types. In fact I already do this in the test suite in a codebase where I have some custom vector types (based on floats and float32s). It's nice because I can just use the same function on any relevant datatype. Cuts down on the noise.

I digress.

---

RE: float naming:

I just realized I had a brain fart in my last comments. float32Close is totally fine with me since we already have floatClose. For some reason I had it in my mind that it was way more verbose -- I think I forgot that it is called floatClose and not just close or something. So yeah my vote is now for float32Close and if everyone else agrees I'm happy to update it.

Float should link to what the CLR / F# considers float. It’s 64 bits in f# I believe. Float32 is that specifically. So three families with no suffix, two with.

Well... C#'s float is a 32 bit float. double is the 64 bit float. So float being float64 is F#'s thing.

---

Sounds like everything else is all good then?

[farlee2121]

Overloads

If you'd like to experiment with overloads, I think the next step would be to implement it on a function or two in a separate branch so we can have a concrete discussion. I'd be happy to collaborate if that's the direction you want to go.

Naming

I'm good with float32Close.

I think the only unresolved action items are naming and potentially splitting the Accuracy constants into two modules. Everything else was good.

[jwosty]

Great - just pushed some changes to split Accuracy into Accuracy and AccuracyF, and rename floatClosef and similar to float32Close.

However in doing this I noticed there's still isNaNf - I think leaving this one makes sense, right? Since there's there's no 'float' word to hang it off of, and the 32 bit NaN literal is called nanf in F#. So someone searching for those would probably expect it to be called isNotNaNf. Same applies for isInfinityf and friends (literal is infinityf).

I also noticed there's no isNaN, isPositiveInfinity, isNegativeInfinity, and isInfinity. I've taken the liberty to add those. It's easy enough to revert that commit if you don't want them.

Overload experiment branch is here: https://github.com/jwosty/expecto/tree/float-overloads-srtp

[farlee2121]

Naming

Hmm. It was my assumption that the nan and inifinity methods would follow a naming pattern similar to the float32Close and Accuracy32. Something like isNan32 and isInfinity32 .

However, you make a good point that the F# constants are nan/nanf and infinity/infinityf.
I agree it would make sense to leave them since they are using the names of language constants.

Overloads

I haven't given this a proper look yet.

Here's the diff: jwosty/expecto@float-overloads...jwosty:expecto:float-overloads-srtp

[farlee2121]

Some thoughts on overloads.

First, thanks for taking the time to put together an example!

I like the GenericMath module. It both separates out the math operations and confines as most of the SRTPs, making it easier to read around the SRTP syntax.

Pros

• The added complexity isn't as bad I thought it might be
• Functions like isNan and is Infinity don't have a somewhat awkward naming convention to accommodate float32

Cons

• The SRTPs definitely make the code harder to read, making it harder for future maintainers to jump in. But it's not as bad I thought it might be.
• The function signature for Expect.close ends up arcane due to the generics. If I was a user trying to figure out what various methods did, I wouldn't be able to parse much from Expect.close's function signature

Some notes so we don't forget if we move forward with overloads

• Expect removed functions like isNanf, but Flip.Expect still has those functions
  • similarly, decide if float32* functions (like float32Close) should still be added

I'm still not certain here. I'd say function signature clarity is pushing me toward separate functions (what we already have) instead of the overloads.

[jwosty]

@farlee2121 good analysis, pretty much agree with everything you've said there.

The function signature for Expect.close ends up arcane due to the generics. If I was a user trying to figure out what various methods did, I wouldn't be able to parse much from Expect.close's function signature

Agreed. The main way to mitigate this is through good documentation with concrete examples -- both external, and inline XML docs. And honestly this is one of the simpler applications of SRTP; we could get really simple and just say "where 'a is float or float32".

(side note - now that I think about it, there's no reason these operators couldn't also just have decimal overloads if we go that direction - but I will leave that for a future PR)

[farlee2121]

I'm a bit unsure on how to understand your comment. Are you still wanting to move forward with the overload approach?

[jwosty]

@farlee2121 If it were my library, I'd probably do the operator overloads. But it is not my library so it's ultimately up to you and can understand wanting to not do the overloads if that's the choice you make

[farlee2121]

Hmm. I want to shy away from seeing it as my library. Expecto has only been possible as a community collaboration.

Also due to the collaborative nature, I'm increasingly leaning toward the more self-documenting separate methods instead of overloads. Keeping the contribution effort and documentation burden as low as we can seems prudent. However, I can certainly be persuaded if anyone wants to speak up for overloads.

[jwosty]

Sure thing. Well, since you're asking :) - my vote would be for the overloads, as reducing noisiness and increasing ergonomics for test code is a big deal IMO. Same reason why other test frameworks Assert classes often heavily rely on overloading. I personally think that, as long as we pay special attention to docs around this part, it outweighs the downside in slightly increased barrier to entry.

[jwosty]

@farlee2121 thoughts? I'd love to get some form of this merged

[farlee2121]

Thanks for the nudge. I was waiting for a tie breaker and then it slipped my attention.

I seem to remember that this PR was in a satisfactory state, but the debate was if we should instead proceed from the branch demonstrating overloads.

Since this PR is already satisfactory, would you be ok if I merge it?