-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Define consistent float ordering #808
Conversation
The downside though is that sorting an array of floats won't give what users probably expect? |
I don’t think anyone has expectation how nans sort. There’s no way to sort arbitrary float arrays without this |
Right, but the bit wise ordering of non-NaN floats isn't the same as the natural ordering? |
Hm, you're right. I think I went too far. You can define equality using bit equality |
509aa2f
to
9f5567f
Compare
fn compare(self, other: Self) -> Ordering; | ||
|
||
fn is_eq(self, other: Self) -> bool; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't these two be expressed with additional trait bounds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can define an additional traits but afaik there isn't one that has the behaviour as implemented here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this just Ord
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok I got it, its about the custom implementation
d899473
to
6bed4fb
Compare
@@ -30,7 +31,6 @@ pub enum PType { | |||
pub trait NativePType: | |||
Send | |||
+ Sync | |||
+ Sized |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why no longer Sized
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it's always Sized https://doc.rust-lang.org/std/marker/trait.Sized.html
We use total_cmp instead of partial_cmp to compare floating point numbers and use bit patern to determine float equality