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

Indices with type-parameterized values #55

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

cscherrer
Copy link

This PR makes a small change to generalize Indices, replacing

mutable struct Indices{I} <: AbstractIndices{I}
    # The hash table
    slots::Vector{Int}

    # Hashes and values
    hashes::Vector{UInt} # Deletion marker stored in high bit
    values::Vector{I}

    holes::Int # Number of "vacant" slots in hashes and values
end

with

mutable struct Indices{I,V} <: AbstractIndices{I}
    # The hash table
    slots::Vector{Int}

    # Hashes and values
    hashes::Vector{UInt} # Deletion marker stored in high bit
    values::V

    holes::Int # Number of "vacant" slots in hashes and values
end

The idea is to allow values stored in other formats, for example a StructArray or TupleVector. I think this will be a step toward being able to implement something like

struct LogWeighted{I, T, V} <: AbstractDictionary{I, T}
    inds::Indices{I,V}

    # keys.data == inds.values
    keys::PermutedVector{V}

    logweights::Vector{T}
    logtotal::LogSumExp{T}

    function LogWeighted{I, T, V}(inds::Indices{I}, values::V, ::Nothing) where {I, T, V<:AbstractVector{T}}
       @assert length(values) == length(_values(inds))
       return new{I,T,V}(inds, values)
    end
end

@cscherrer cscherrer changed the title Inidices with type-parameterized values Indices with type-parameterized values Jul 12, 2021
@andyferris
Copy link
Owner

andyferris commented Jul 12, 2021

Thanks @cscherrer.

This also seems natural with Dictionary and its values. It's worth stating that a downside to this is that there is annoying increase in verbosity to the types, compared to Base.Set and Base.Dict. This could possibly be severe enough that it is worth having this functionality as an entirely new type, even. (For context, I still consider much of this work as prototyping behavior that potentially could be ported to Julia 2.0, so we need a similar level of polish and simplicity as Base).

The one thing we have at the moment that I'd really like to preserve is that we can state whether a dictionary is issettable or isinsertable. Now this will depend on the underlying arrays. Before moving forward I think we need to make a plan for this. To handle arrays and dictionaries gracefully, I have been considering the following body of work:

  • Finish Add similar_type #54 including the new and new_type functions (note - names are placeholders, basically these are generalized versions of collect except in new(T, values) the T is the container type not the element type as in collect(T, values))
  • Implement new_type, similar_type and empty_type for arrays.
  • Implement isinsertable and issettable for arrays
  • Consider if the above belongs in a seperate mini package.
  • We can now define isinsertable(inds::Indices) = isinsertable(inds.values), etc.

Any thoughts around this would be appreciated. (CC @c42f).

@andyferris
Copy link
Owner

Also - we currently have a contrib directory where stuff people want to play with could live.

I think we could rename this to src/experimental, with the idea that we'd accept all sorts of containers-in-progress and experimental APIs there, and have them as unexported types and docstrings saying they are experimental. The unordered hash dictionary for example is still quite useful. We'll need to create a WeakKeyDictionary. I want to make sure the token interface works with sorted dictionaries (trees and such).

@cscherrer
Copy link
Author

Thanks @andyferris, I appreciate the care you're taking in thinking through this. I wasn't aware of the Julia 2.0 plan, but that sounds like it could work out great :)

Putting it in contrib or experimental would be fine for me. Any ideas for a name for this? It more abstract than Indices, but still a concrete type, so I'm not sure what to call it.

@andyferris
Copy link
Owner

I wasn't aware of the Julia 2.0 plan

To be clear this is more of a personal wish than an agreed-upon plan :)

I haven't thought of any good name. CustomIndices? GenericIndices? Also, feel free to use a single-character prefix.

@andyferris
Copy link
Owner

Another approach is to add CustomIndices (or whatever) and maintain an exported const Indices{I} = CustomIndices{I, Vector{I}}?

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.

2 participants