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

Make OffsetArrays a stdlib #45585

Closed
LilithHafner opened this issue Jun 4, 2022 · 4 comments
Closed

Make OffsetArrays a stdlib #45585

LilithHafner opened this issue Jun 4, 2022 · 4 comments
Labels
stdlib Julia's standard library

Comments

@LilithHafner
Copy link
Member

I think that it would be nice to have OffsetArrays as a stdlib.

My particular use case that requires it is in sorting. It is sometimes necessary/helpful for performance to have a workspace AbstractVector with the same indices as the input or the region of the input that is to be sorted. For example, radix-sort copies the whole vector back and forth, each time putting it in a slightly more ordered order. A new, faster, QuickSort would do the same. To have any chance of depending on OffsetArrays, OffsetArrays must be a stdlib.

It would be nice to have a unified way of reusing workspaces (e.g. a Vector{UInt8} that sorting methods resize! and reinterpret as needed). When the input is an AbstractVector with offset indices, we could simply construct a workspace with OffsetVector(reinterpret(eltype(input), workspace), firstindex(input)-1).

Without OffsetArrays, this becomes less elegant. Consider, for example, the case when the input is an OffsetArray with negative firstindex. We would need to sort a view into the input that discards the offset indices. This is problematic for two reasons:

  1. It is more elegant to support arbitrary indexing wherever possible rather than explicitly convert to one-based indexing
  2. views have non-negligible overhead in some of these cases

similar is not sufficient because the workspace must support resizing, should be reinterpretable from a Vector{UInt8}, and should support indices other than the indices of the input (e.g. MergeSort only needs half the size, sorting a Matrix needs much less).

@LilithHafner LilithHafner added the stdlib Julia's standard library label Jun 4, 2022
@LilithHafner
Copy link
Member Author

Pairs with #45584 to enable #45570 and #45222

@N5N3
Copy link
Member

N5N3 commented Jun 5, 2022

Can't we just add a simple wrapper in sort.jl which transforms the input offsetvector to a 1-based one? (We only need to cache the firstindex)

@LilithHafner
Copy link
Member Author

Yes. We can do that with view(input, firstindex(input):lastindex(input)). I'll pursue that and report back if I run into performance overhead from those views.

@LilithHafner
Copy link
Member Author

Ick. I don't like stdlibs anymore.

@LilithHafner LilithHafner closed this as not planned Won't fix, can't repro, duplicate, stale Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Julia's standard library
Projects
None yet
Development

No branches or pull requests

2 participants