[FR] Implement hashing

[marius311]

Hashing ComponentArrays (v0.9.3) doesn't seem to be implemented and the generic fallback (tested on 1.6) doesn't work. An unfortunately side-effect of this is that ComponentArrays don't work with certain remote constructs (here because auto global-shipping relies on the hash):

julia> using ComponentArrays, Distributed

julia> addprocs(1);

julia> x = ComponentArray(a=[1,2], b=[3]);

julia> @fetch x
ERROR: MethodError: no method matching LinearIndices(::Tuple{Symbol, Symbol})
Closest candidates are:
  LinearIndices(::Tuple{}) at indices.jl:454
  LinearIndices(::R) where {N, R<:Tuple{Vararg{AbstractUnitRange{Int64}, N}}} at indices.jl:451
  LinearIndices(::Tuple{Vararg{AbstractUnitRange{var"#s79"} where var"#s79"<:Integer, N}}) where N at indices.jl:455
  ...
Stacktrace:
  [1] hash(A::ComponentVector{Int64}, h::UInt64)
...

[jonniedie]

Hm. I'd love to have this working, but to be honest, I have no idea what I'm supposed to do to implement this. Looking at how it's implemented for AbstractArrays doesn't help much either. Can I just do something like this?

Base.hash(x::ComponentArray, h::UInt) = hash(getdata(x), h)

The docstring for hash mentions that == should be implemented as well, but it seems like the fallback ==(A::AbstractArray, B::AbstractArray) should be sufficient.

[marius311]

I guess its a little unclear because the docstring says

Compute an integer hash code such that isequal(x,y) implies hash(x)==hash(y). 

and we currently have ComponentArray(a=1, b=2) == [1,2] so it seems like your hash function is the right one, but at the same time, that would imply (to use remote shipping as an example, but anything bashed on hash would behave similarly)

julia> x = [1,2];

julia> @fetch x
[1,2]

julia> x = ComponentArray(a=1, b=2);

julia> @fetch x
[1,2] # x not updated on remote since it thinks it hasnt changed locally

Perhaps its actually ComponentArray(a=1, b=2) == [1,2] that should be false? What do other array wrapper packages do?

[jonniedie]

All the other array wrappers that I've tried (including stdlib types like Adjoint and SubArray) work the same way

julia> [1 2 3] == [1, 2, 3]'
true

julia> [1, 2] == view([1, 2, 3], 1:2)
true

julia> using StaticArrays

julia> SVector(1, 2, 3) == [1, 2, 3]
true

julia> using LabelledArrays

julia> LVector(a=1, b=2, c=3) == [1, 2, 3]
true

It seems like hashing on the underlying array is the right thing to do, then, to be consistent with everyone else.

julia> hash([1 2 3]) == hash([1, 2, 3]')
true

julia> hash([1, 2]) == hash(view([1, 2, 3], 1:2))
true

julia> hash(SVector(1,2,3)) == hash([1,2,3])
true

[jonniedie]

And StaticArrays works because when it falls back to the default AbstractArray method, keys gives the LinearIndices of the array so it will work the same as a plain array.

[marius311]

Ah just saw you merged. Unfortunately I have to say, I think this is the wrong behaviour and I do think ComponentArrays should implement its own == (and hash) which accounts for the axes. To me the axes are a fundamental part of what they are, so the current behaviour is wierd

julia> ComponentArray(a=1, b=1) == ComponentArray(x=1, z=1)
true

This is similarly wrong to me:

julia> component_array1 == component_array2
true

julia> @unpack a = component_array1
1

julia> @unpack a = component_array2
# error

If equality included the axes, then so would hash (solving the distributed issue above), and it just seems more natural. If someone wants to check equality vs an array, the can do all(component_array .== array).

[jonniedie]

I still haven't tagged it yet so I could change it again for this release. I agree that it would make more sense for equality to require the same axes as well as underlying data.

With that said, I am hesitant to implement behavior that is fundamentally different than all the other array types. I don't really know whose code is depending on equivalence of two different types of arrays in this way.

Also, the same argument as yours above could be made for, say 1 and 1+0im:

julia> x1 = 1; x2 = 1+0im;

julia> x1 == x2
true

julia> hash(x1) == hash(x2)
true

julia> @unpack re = x2
1 + 0im

julia> @unpack re = x1
ERROR: type Int64 has no field re
Stacktrace:
 [1] getproperty(x::Int64, f::Symbol)
   @ Base .\Base.jl:33
 [2] unpack(x::Int64, #unused#::Val{:re})
   @ UnPack ~\.julia\packages\UnPack\EkESO\src\UnPack.jl:34
 [3] top-level scope
   @ ~\.julia\packages\UnPack\EkESO\src\UnPack.jl:101

If I were able to just change ==(a::ComponentArray, b::ComponentArray) so they had to have matching axes and let ==(a::ComponentArray, b::AbstractArray) fall back to the default method, I would, but I don't see any way to make this work with hashing.

[jonniedie]

I’ll reopen the issue too.

[jonniedie]

I guess the counterpoint to my last example would be:

julia> nt1 = (a=1, b=1); nt2 = (x=1, z=1);

julia> nt1 == nt2
false

[marius311]

Hmm yea that last example I think puts it in perspective. I guess there's basically just two incompatible interfaces in terms of what ==/hash should give, AbstractArrays and NamedTuples, and the question is which one is a ComponentArray? Even though intuitively I think of it as "both," and its NamedTuple-ness as kind of a superset of its Array-ness, I suppose its formally an <:AbstractArray, so maybe you did make the right choice. I'm fine if you wanted to mark it as closed again. Might be interesting to ask on Slack/Discourse to get other inputs, I bet this has come up before but I don't know where.

[jonniedie]

I think maybe a good way to frame this is that ComponentArrays are typically used as NamedTuples by end users and AbstractArrayss by the solvers they are passing them to. Users always know that they are working with ComponentArrays, so they at least have the ability to change their code accordingly. The authors of solver libraries (DifferentialEquations, Optim, Turing, and wherever else ComponentArrays are used) don't--and shouldn't need to--know that their code is working with ComponentArrays.

So I think I'd rather err on the side of AbstractArray-ness. If a solver uses A==B somewhere expecting the arrays A and B to follow the standard convention for equality of AbstractArrays, the user who is trying to pass a ComponentArray through that solver is going to have a problem that they can't fix without some serious monkey-patching. On the other hand, if a user needs ComponentArrays to follow a more strict NamedTuple-ish form of equality, they can always do themselves with something like

is_strictly_equal(a::A, b::B) where {A,B} = a==b && A===B

Actually, maybe it would be a good idea to just define that in ComponentArrays and export it. And maybe give a warning when equality is asked between two equal but not strictly equal ComponentArrays.

[jonniedie]

Alright, let's do it the ComponentArray(a=1, b=1) != ComponentArray(x=1, z=1) way.

[marius311]

Thanks for looking into this, I agree in the end this was the right solution!