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

IntSet: O(1) maximum/minimum/issorted #22856

Merged
merged 1 commit into from
Jul 20, 2017
Merged
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions base/intset.jl
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,12 @@ done(s::IntSet, i) = i <= 0


@noinline _throw_intset_notempty_error() = throw(ArgumentError("collection must be non-empty"))

function first(s::IntSet)
idx = findfirst(s.bits)
idx == 0 ? _throw_intset_notempty_error() : idx
end

function last(s::IntSet)
idx = findprev(s.bits, length(s.bits))
idx == 0 ? _throw_intset_notempty_error() : idx
Expand Down Expand Up @@ -239,3 +245,8 @@ function hash(s::IntSet, h::UInt)
end
h
end

minimum(s::IntSet) = first(s)
maximum(s::IntSet) = last(s)
extrema(s::IntSet) = (first(s), last(s))
issorted(s::IntSet) = true
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

What does this mean for a Set?

Copy link
Member Author

Choose a reason for hiding this comment

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

Like for generic iterators: if the values yielded by iteration are sorted.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Okay, the documentation specifically says that it checks if a vector is sorted. Should that be loosened?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch! yes I would say it should be loosened, as long as the method is defined for generic iterators (which makes sense for me).

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Is the order a set is iterated in even defined i. e does the order have to be the same every time even if the set is unchanged? Defining issorted for something unordered feels a bit iffy and perhaps mixes implementation with public API.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is the order a set is iterated in even defined

I guess yes for Set, but for IntSet it's always in sorted order, and I believe it will stay this way. I don't know whether it's documented though. This added method here is only an optimization.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Same as the situation for Dict, SortedDict, and OrderedDict. Some sets might be sorted, others not.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Yes but is the order that Sets and IntSets are iterated guaranteed to stay fixed? Otherwise being sorted is not a property of the object. For the other structures you mentioned, there is clearly a well defined iteration order so issorted makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

are any of these tested?

Copy link
Member Author

Choose a reason for hiding this comment

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

For IntSet, the iteration order is guaranteed to stay fixed; if the implementation changes, then the issorted method will have to be updated accordingly.