cleanups - #92
Conversation
simonbyrne
left a comment
There was a problem hiding this comment.
That's a much better idea, I don't know why I didn't do it this way originally.
|
Also, while I'm cleaning things up, Is the current approach of calling C functions for the basic conversions good? I see ~2x faster performance using the pure Julia UInt128 conversion than the ccalled UInt64 conversion (zen5 linux). If one of you has an aarc64 system (which has native Float128), can you run this benchmark? |
|
On my M2 Mac Yeah, so that looks faster... |
|
@simonbyrne I might have gone slightly overboard and moved a lot of the simple implementations into Julia with pretty decent perf gains (usually ~2x sometimes as big as 8x) |
|
so, should we merge this? |
|
I'll defer to @simonbyrne |
|
I'll say. Simon very nice grist for Float128. Floats where the edge [cases] have been honed to refract |
|
Let's bump the minor version (or make it 1.0) then it's good |
|
Lets go to 1.0! |
|
I do not agree with going to 1.0 until a few other issues are addressed. |
|
That's easy enough to fix. Would printing as |
|
|
Are we willing to rely on Base.Ryu for this? it already has the code and it works really well. |
|
Also, how would we feel about printing/showing these as |
|
"Also, how would we feel about printing/showing these as Inf and NaN rather than Inf128 and NaN128? That would match what we do for Inf32 and other 32 bit types." |
|
I pushed the changes, and then immediately tested with a non trivial case and it broke (Base.Ryu is fairly non generic and requires a bunch of internal functions to be defined to work). That said, I don't love the snprintf printing either since it doesn't provide a way of doing shortest printing to match Julia. I've moved it a bit closer to Julia printing now, but there are still a few quirks. For example |
|
so... merge? |
This removes a bunch of compats for unsupported Julia versions, and slightly simplifies the annoying hacks we're making to handle the ABI issues.