fsprojects / SIMDArray

SIMD enhanced Array operations
MIT License
132 stars 15 forks source link

Handle NaN in min/max/minBy/maxBy #22

Closed marklam closed 7 years ago

marklam commented 7 years ago

Some caveats:

marklam commented 7 years ago

This should fix Issue #21 (provided it doesn't hurt performance too badly)

jackmott commented 7 years ago

I'm wondering if it is a good idea to compromise performance to handle nan/infinity the same as the standard lib functions. I'll do some testing to see how big the effect is.

jackmott commented 7 years ago

What do you think about just modifying the comments to say that NaN and infinity are handled differently? I'm not sure how users of the lib would prefer things.

jackmott commented 7 years ago

This is really nice. I feel like there might be some casting trick we could pull to let minBy and maxBy still return ^U. The bits would work, just need to convince the compiler =)

marklam commented 7 years ago

I wouldn't mind if the results for NaN/infnity were different - I'd be upset about having them in the data in the first place :-) And it's not like the change from Array.max -> Array.SIMD.max is silent.

I think that losing the ^U mapping is necessary when the Vector.Count is different for ^T and ^U.

Not sure why that last commit won't build on the CI though, it built fine on my machine (debug and release).

jackmott commented 7 years ago

Yeah even if we kept the ^T and ^U I would need to add a runtime check for the Count being the same.

I'll merge this and see if it builds for me.

jackmott commented 7 years ago

does't build on my machine either, MinValue.Invoke() returns an object, and then the compiler doesn't know what to do with Vector< ^T>(minValue)

jackmott commented 7 years ago

looks like just needs type annotations.

marklam commented 7 years ago

I wonder what the difference is - that I'm using the Visual F# nightly, maybe?

jackmott commented 7 years ago

possibly F# 4.1 can handle it but not 4.0?

jackmott commented 7 years ago

I pushed up a fix