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

sort handles incomparable values #5998

Merged
merged 56 commits into from
Apr 16, 2023
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
299c9d4
Update type ascriptions in some operators in Any
Akirathan Mar 17, 2023
b8a9c9e
Add @GenerateUncached to AnyToTextNode.
Akirathan Mar 17, 2023
38b6b8a
Add tests for "sort handles incomparable types"
Akirathan Mar 17, 2023
3f66d8a
Vector.sort handles incomparable types
Akirathan Mar 17, 2023
e57c4ae
Implement sort handling for different comparators
Akirathan Mar 20, 2023
603ab95
Comparison operators in Any do not throw Type_Error
Akirathan Mar 21, 2023
95c0878
Fix some issues in Ordering_Spec
Akirathan Mar 21, 2023
0df66b1
Remove the remaining comparison operator overrides for numbers.
Akirathan Feb 10, 2023
b5692b8
Consolidate all sorting functionality into a single builtin node.
Akirathan Mar 22, 2023
75d3178
Fix warnings attachment in sort
Akirathan Mar 24, 2023
f8deb71
PrimitiveValuesComparator handles other types than primitives
Akirathan Mar 24, 2023
82ac86b
Merge branch 'develop' into wip/akirathan/sort-incomparable-5742
Akirathan Mar 24, 2023
e695e1f
Fix byFunc calling
Akirathan Mar 28, 2023
219aa4a
on function can be called from the builtin
Akirathan Mar 28, 2023
38057d7
Merge branch 'develop' into wip/akirathan/sort-incomparable-5742
Akirathan Mar 28, 2023
8bd3e00
Merge branch 'develop' into wip/akirathan/sort-incomparable-5742
Akirathan Apr 3, 2023
1360be6
Fix build of native image
Akirathan Apr 3, 2023
4fbdc0d
Update changelog
Akirathan Apr 3, 2023
8dfb873
Add VectorSortTest
Akirathan Apr 5, 2023
1528446
Merge branch 'develop' into wip/akirathan/sort-incomparable-5742
Akirathan Apr 5, 2023
80c69d0
Builtin method should not throw DataflowError.
Akirathan Apr 5, 2023
e0881f4
TypeOfNode may not return only Type
Akirathan Apr 5, 2023
19d2d6f
UnresolvedSymbol is not supported as `on` argument to Vector.sort_bui…
Akirathan Apr 5, 2023
9278734
Fix docs
Akirathan Apr 5, 2023
af9dfa3
Fix bigint spec in LessThanNode
Akirathan Apr 5, 2023
7f2af25
Small fixes
Akirathan Apr 5, 2023
ed97b04
Small fixes
Akirathan Apr 5, 2023
c726753
Nothings and Nans are sorted at the end of default comparator group.
Akirathan Apr 5, 2023
92a10a8
Fix checking of `by` parameter - now accepts functions with default a…
Akirathan Apr 5, 2023
8f78728
Fix changelog formatting
Akirathan Apr 6, 2023
e2075d9
Fix imports in DebuggingEnsoTest
Akirathan Apr 6, 2023
6983769
Remove Array.sort_builtin
Akirathan Apr 6, 2023
85be93d
Merge branch 'develop' into wip/akirathan/sort-incomparable-5742
Akirathan Apr 6, 2023
a2bfc8b
Add comparison operators to micro-distribution
Akirathan Apr 6, 2023
310f1df
Remove Array.sort_builtin
Akirathan Apr 12, 2023
fee458a
Replace Incomparable_Values by Type_Error in some tests
Akirathan Apr 12, 2023
7f933dd
Add on_incomparable argument to Vector.sort_builtin
Akirathan Apr 13, 2023
ddcdacb
Merge branch 'develop' into wip/akirathan/sort-incomparable-5742
Akirathan Apr 13, 2023
08f0e31
Fix after merge - Array.sort delegates to Vector.sort
Akirathan Apr 13, 2023
5bc344e
Add more tests for problem_behavior on Vector.sort
Akirathan Apr 13, 2023
a8091d1
SortVectorNode throws only Incomparable_Values.
Akirathan Apr 13, 2023
d4b487e
Delete Collections helper class
Akirathan Apr 13, 2023
b39ea29
Add test for expected failure for custom incomparable values
Akirathan Apr 13, 2023
2a41993
Cosmetics.
Akirathan Apr 13, 2023
9e04935
Fix test expecting different comparators warning
Akirathan Apr 13, 2023
cc1f527
isNothing is checked via interop
Akirathan Apr 14, 2023
715fe94
Remove TruffleLogger from SortVectorNode
Akirathan Apr 14, 2023
8268e02
Small review refactorings
Akirathan Apr 14, 2023
254d579
Revert "Remove the remaining comparison operator overrides for numbers."
Akirathan Apr 15, 2023
8174ca6
Improve bench_download.py tool's `--compare` functionality.
Akirathan Apr 15, 2023
672afb0
Wrap potential interop values with `HostValueToEnsoNode`
Akirathan Apr 15, 2023
6b8f147
Use alter function in Vector_Spec
Akirathan Apr 15, 2023
d9a12a0
Update docs
Akirathan Apr 15, 2023
8718c99
Merge branch 'develop' into wip/akirathan/sort-incomparable-5742
Akirathan Apr 15, 2023
1f23d83
Invalid comparison throws Incomparable_Values rather than Type_Error
Akirathan Apr 15, 2023
ab2f95a
Number comparison builtin methods return Nothing in case of incompara…
Akirathan Apr 16, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,7 @@
- [Detect potential name conflicts between exported types and FQNs][5966]
- [Ensure calls involving warnings remain instrumented][6067]
- [One can define lazy atom fields][6151]
- [Vector.sort handles incomparable types][5998]

[3227]: https://github.com/enso-org/enso/pull/3227
[3248]: https://github.com/enso-org/enso/pull/3248
Expand Down Expand Up @@ -777,6 +778,7 @@
[5966]: https://github.com/enso-org/enso/pull/5966
[6067]: https://github.com/enso-org/enso/pull/6067
[6151]: https://github.com/enso-org/enso/pull/6151
[5998]: https://github.com/enso-org/enso/pull/5998

# Enso 2.0.0-alpha.18 (2021-10-12)

Expand Down
6 changes: 3 additions & 3 deletions distribution/lib/Standard/Base/0.0.0-dev/src/Any.enso
Original file line number Diff line number Diff line change
Expand Up @@ -453,11 +453,11 @@ type Any

## PRIVATE
Checks if the comparators for the given objects are both of the same type. If so,
proceeds with the given action, and if not, throws `Type_Error`.
assert_same_comparators : Any -> Any -> (Any -> Any) -> Any ! Type_Error
proceeds with the given action, and if not, throws `Incomparable_Values` error.
assert_same_comparators : Any -> Any -> (Any -> Any) -> Any ! Incomparable_Values
assert_same_comparators this that ~action =
comp_this = Comparable.from this
comp_that = Comparable.from that
case Meta.is_same_object comp_this comp_that of
True -> action
False -> Error.throw (Type_Error.Error (Meta.type_of comp_this) comp_that "comp_that")
False -> Error.throw (Incomparable_Values.Error this that)
15 changes: 0 additions & 15 deletions distribution/lib/Standard/Base/0.0.0-dev/src/Data/Array.enso
Original file line number Diff line number Diff line change
Expand Up @@ -135,21 +135,6 @@ type Array
length : Integer
length self = @Builtin_Method "Array.length"

## Sorts the Array.

Arguments:
- comparator: A comparison function that takes two elements and returns
an Ordering that describes how the first element is ordered with
respect to the second.

> Example
Getting a sorted array of numbers.

[3,2,1].to_array.sort
sort : (Any -> Any -> Ordering) -> Array
sort self comparator=(Ordering.compare _ _) =
self.sort_builtin comparator

## Identity.
This method is implemented purely for completeness with the runtime's
primitive array protocol.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ invert_range_selection ranges length needs_sorting =
Empty subranges are discarded.
sort_and_merge_ranges : Vector Range -> Vector Range
sort_and_merge_ranges ranges =
sorted = ranges.filter (range-> range.is_empty.not) . sort on=(.start)
sorted = ranges.filter (range-> range.is_empty.not) . sort on=(_.start)
Copy link
Member Author

Choose a reason for hiding this comment

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

(.start) is parsed to UnresolvedSymbol<start>, and I found no easy way how to call that. Whereas (_.start) is parsed into Function. I created issue #6726 to track that.

Copy link
Member

Choose a reason for hiding this comment

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

#6276 *

if sorted.is_empty then [] else
current_ref = Ref.new sorted.first
builder = Vector.new_builder
Expand Down
100 changes: 3 additions & 97 deletions distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numbers.enso
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import project.Data.Text.Text
import project.Data.Locale.Locale
import project.Errors.Common.Arithmetic_Error
import project.Errors.Common.Incomparable_Values
import project.Error.Error
import project.Nothing.Nothing
import project.Panic.Panic
Expand Down Expand Up @@ -288,7 +289,7 @@ type Number
Check if 1 is equal to 1.0000001 within 0.001.

1.equals 1.0000001 epsilon=0.001
equals : Number -> Number -> Boolean
equals : Number -> Number -> Boolean ! Incomparable_Values
equals self that epsilon=0.0 =
(self == that) || ((self - that).abs <= epsilon)
Akirathan marked this conversation as resolved.
Show resolved Hide resolved

Expand Down Expand Up @@ -457,54 +458,6 @@ type Decimal
^ : Number -> Number
^ self that = @Builtin_Method "Decimal.^"

## Checks if this is greater than that.
JaroslavTulach marked this conversation as resolved.
Show resolved Hide resolved

Arguments:
- that: The number to compare this against.

> Example
Checking if 10 is greater than 7.3.

10 > 7.3
> : Number -> Boolean
> self that = @Builtin_Method "Decimal.>"

## Checks if this is greater than or equal to thatthat.

Arguments:
- that: The number to compare this against.

> Example
Checking if 10 is greater than or equal to 7.3.

10 >= 7.3
>= : Number -> Boolean
>= self that = @Builtin_Method "Decimal.>="

## Checks if this is less than that.

Arguments:
- that: The number to compare this against.

> Example
Checking if 10 is less than 7.3.

10 < 7.3
< : Number -> Boolean
< self that = @Builtin_Method "Decimal.<"

## Checks if this is less than or equal to thatthat.

Arguments:
- that: The number to compare this against.

> Example
Checking if 10.4 is less than or equal to 7.

10.4 <= 7
<= : Number -> Boolean
<= self that = @Builtin_Method "Decimal.<="

## Computes the absolute value of this.

The absolute value of a positive number is itself, while the absolute
Expand Down Expand Up @@ -682,54 +635,6 @@ type Integer
^ : Number -> Number
^ self that = @Builtin_Method "Integer.^"

## Checks if this is greater than that.

Arguments:
- that: The number to compare this against.

> Example
Checking if 10 is greater than 7.

10 > 7
> : Number -> Boolean
> self that = @Builtin_Method "Integer.>"

## Checks if this is greater than or equal to thatthat.

Arguments:
- that: The number to compare this against.

> Example
Checking if 10 is greater than or equal to 7.

10 >= 7
>= : Number -> Boolean
>= self that = @Builtin_Method "Integer.>="

## Checks if this is less than that.

Arguments:
- that: The number to compare this against.

> Example
Checking if 10 is less than 7.

10 < 7
< : Number -> Boolean
< self that = @Builtin_Method "Integer.<"

## Checks if this is less than or equal to thatthat.

Arguments:
- that: The number to compare this against.

> Example
Checking if 10 is less than or equal to 7.

10 <= 7
<= : Number -> Boolean
<= self that = @Builtin_Method "Integer.<="

## Computes the absolute value of this.

The absolute value of a positive number is itself, while the absolute
Expand Down Expand Up @@ -937,6 +842,7 @@ type Integer

parse_builtin text radix = @Builtin_Method "Integer.parse"


## UNSTABLE

A syntax error when parsing a double.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ from project.Data.Boolean import all

```
type Comparator T
compare : T -> T -> (Ordering|Nothing)
compare : T -> T -> (Ordering|Nothing) ! Incomparable_Values
Akirathan marked this conversation as resolved.
Show resolved Hide resolved
hash : T -> Integer
```

Expand Down Expand Up @@ -170,7 +170,7 @@ type Ordering
Greater

## Compares to values and returns an Ordering
compare : Any -> Any -> Ordering ! (Incomparable_Values | Type_Error)
compare : Any -> Any -> Ordering ! Incomparable_Values
Akirathan marked this conversation as resolved.
Show resolved Hide resolved
compare x y =
if x < y then Ordering.Less else
if x == y then Ordering.Equal else
Expand Down
58 changes: 42 additions & 16 deletions distribution/lib/Standard/Base/0.0.0-dev/src/Data/Vector.enso
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import project.Data.Filter_Condition.Filter_Condition
import project.Data.List.List
import project.Data.Map.Map
import project.Data.Numbers.Integer
import project.Data.Ordering.Ordering
import project.Data.Ordering.Sort_Direction.Sort_Direction
import project.Data.Pair.Pair
import project.Data.Range.Range
Expand All @@ -19,10 +18,17 @@ import project.Error.Error
import project.Errors.Illegal_Argument.Illegal_Argument
import project.Function.Function
import project.Math
import project.Meta
import project.Nothing.Nothing
import project.Panic.Panic
import project.Random
import project.Warning.Warning

import project.IO

# We have to import also conversion methods, therefore, we import all from the Ordering
# module
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# We have to import also conversion methods, therefore, we import all from the Ordering
# module
## We have to import also conversion methods, therefore, we import all from the Ordering
module

I think the convention is to use ## for the multiline comments. Unless there was some issue with it?

Copy link
Member

Choose a reason for hiding this comment

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

Btw. I think this raises a good point that we may want to have a way to import conversion methods without necessarily importing all. What do you think? Shall we start a discussion?

I guess one way to do so should be to do import project.Data.Ordering instead. I'm pretty sure the not-from imports do import the extension methods within.

Copy link
Member

Choose a reason for hiding this comment

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

But maybe a more explicit way for this would be good too. Just a random thought, low priority. Definitely independent of this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Shall we start a discussion?

I don't know. I guess it depends on whether the current behavior is the expected one, i.e., if I import a type with import Standard.Base.Data.Ordering.Comparable.Comparable, then all its extension methods from that module are imported as well, but not any conversion methods - neither those with Comparable as target, neither those with Comparable as source. Is this intuitive/expected/bug?

Copy link
Member

Choose a reason for hiding this comment

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

Honestly, I don't know.

My intuition would be that if I import a type, probably it's extension methods are imported too... But probably only its and not others.

I'd appreciate a way to import only extension methods from a file, but that's just my subjective view. Also definitely not something of high priority, as we are doing fine with current state of the import system, I think.

from project.Data.Ordering import all
from project.Data.Boolean import Boolean, True, False
from project.Data.Index_Sub_Range import Index_Sub_Range, take_helper, drop_helper

Expand Down Expand Up @@ -841,13 +847,16 @@ type Vector a
Arguments:
- order: The order in which the vector elements are sorted.
- on: A projection from the element type to the value of that element
being sorted on.
being sorted on. If set to `Nothing` (the default argument),
identity function will be used.
- by: A function that compares the result of applying `on` to two
elements, returning an Ordering to compare them.
elements, returning an an `Ordering` if the two elements are comparable
or `Nothing` if they are not. If set to `Nothing` (the default argument),
`Ordering.compare _ _` method will be used.
Must be a static method.

By default, elements are sorted in ascending order.

By default, elements are sorted in ascending order, using the comparator
acquired from each element. A custom compare function may be passed to
the sort method.

This is a stable sort, meaning that items that compare the same will not
have their order changed by the sorting process.
Expand All @@ -865,6 +874,18 @@ type Vector a
is partially sorted. When the vector is randomly ordered, the
performance is equivalent to a standard mergesort.

? Multiple comparators
Elements with different comparators are incomparable by definition.
This case is handled by first grouping the `self` vector into groups
with the same comparator, recursively sorting these groups, and then
merging them back together. The order of the sorted groups in the
resulting vector is based on the order of fully qualified names of
the comparators in the `self` vector, with the exception of the group
for the default comparator, which is always the first group.

Additionally, a warning will be attached, explaining that incomparable
values were encountered.

It takes equal advantage of ascending and descending runs in the array,
making it much simpler to merge two or more sorted arrays: simply
concatenate them and sort.
Expand All @@ -878,16 +899,21 @@ type Vector a
Sorting a vector of `Pair`s on the first element, descending.

[Pair 1 2, Pair -1 8].sort Sort_Direction.Descending (_.first)
sort : Sort_Direction -> (Any -> Any) -> (Any -> Any -> Ordering) -> Vector Any ! Incomparable_Values
sort self (order = Sort_Direction.Ascending) (on = x -> x) (by = (Ordering.compare _ _)) =
comp_ascending l r = by (on l) (on r)
comp_descending l r = by (on r) (on l)
compare = if order == Sort_Direction.Ascending then comp_ascending else
comp_descending

new_vec_arr = self.to_array.sort compare
if new_vec_arr.is_error then Error.throw new_vec_arr else
Vector.from_polyglot_array new_vec_arr

> Example
Sorting a vector with elements with different comparators. Values 1
and My_Type have different comparators. 1 will be sorted before My_Type
because it has the default comparator, and warning will be attached to
the resulting vector.

[My_Type.Value 'hello', 1].sort == [1, My_Type.Value 'hello']
sort : Sort_Direction -> (Any -> Any)|Nothing -> (Any -> Any -> (Ordering|Nothing))|Nothing -> Vector Any ! Incomparable_Values
sort self (order = Sort_Direction.Ascending) on=Nothing by=Nothing =
comps = case on == Nothing of
True -> self.map it-> Comparable.from it
False -> self.map it-> Comparable.from (on it)
compare_funcs = comps.map (it-> it.compare)
self.sort_builtin order.to_sign comps compare_funcs by on

## UNSTABLE
Keeps only unique elements within the Vector, removing any duplicates.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ from Standard.Base import all
from project import Test
import project.Extensions

## Returns values of warnings attached to the value.Nothing
radeusgd marked this conversation as resolved.
Show resolved Hide resolved
## Returns values of warnings attached to the value.
get_attached_warnings v =
Warning.get_all v . map .value

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ private HostEnsoUtils() {
/**
* Extracts a string representation for a polyglot exception.
*
* @param exexception the exception
* @param ex the exception
* @return message representing the exception
*/
public static String findExceptionMessage(Throwable ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.dsl.Fallback;
import com.oracle.truffle.api.dsl.GenerateUncached;
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.interop.InteropLibrary;
import com.oracle.truffle.api.interop.UnsupportedMessageException;
Expand All @@ -26,6 +27,7 @@
name = "type_of",
description = "Returns the type of a value.",
autoRegister = false)
@GenerateUncached
public abstract class TypeOfNode extends Node {

public abstract Object execute(@AcceptsError Object value);
Expand Down
Loading