Skip to content
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

Don't use totality when defining the (potentially) partial order relation <= #22027

Merged
merged 1 commit into from
May 24, 2017

Conversation

andreasnoack
Copy link
Member

No description provided.

@@ -97,3 +97,15 @@ end
B = 3 .> [1 -1 5] .> 0
@test B == [true false false]
end

Base.:(<)(x::Type, y::Type) = (x <: y) & (x != y)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be wrapped in a custom type e.g. struct SortType x end. While no other test should be relying on this, there could be future interference should another test want to do something similar.

@alyst
Copy link
Contributor

alyst commented May 23, 2017

Potentially it's 2 comparisons vs 1. What would nanosoldier say?

@andreasnoack
Copy link
Member Author

Potentially it's 2 comparisons vs 1. What would nanosoldier say?

For integers, the generated code is very similar, but why guess when you can just ask

@nanosoldier runbenchmarks(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

@andreasnoack andreasnoack merged commit a1ffb9e into master May 24, 2017
@andreasnoack andreasnoack deleted the anj/comp branch May 24, 2017 00:58
@StefanKarpinski
Copy link
Member

This came up on discourse: https://discourse.julialang.org/t/behavior-of-when-isless-or-is-defined/10114. Looking at it again, I'm wondering what the motivation for the change was. As a general fallback since even if this doesn't cause a performance problem for built-in types, user-defined types may have expensive < and == operations and this always calls them both.

@andreasnoack
Copy link
Member Author

The main motivation was correctness. We state that < is a partial order relation so it seemed a bit odd to use totality here. I think fallbacks should be correct and special cases optimized. The problem is that it is not easy to communicate that users might benefit from defining special <= for their type.

Just thinking out loud a possible alternative: my impression is that the isless/< thing is mainly there because of floats and not because of some deeper order theory considerations. Hence an alternative could be that < should generally be a total order and then clearly state that floating points are special and has a very special isless function that are not supposed to be used for anything but floats.

@StefanKarpinski
Copy link
Member

I do agree that < being a partial order is a bit fishy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants