Skip to content

Nested structs broken #126

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

Closed
SimonDanisch opened this issue May 5, 2020 · 4 comments
Closed

Nested structs broken #126

SimonDanisch opened this issue May 5, 2020 · 4 comments

Comments

@SimonDanisch
Copy link
Member

This should (was) working:

struct PointX3f0
       data::NTuple{3, Float32}
end

StructArray{PointX3f0}(([1f0], [1f0], [1f0]))

If not a bug, how can we make it work?

@piever
Copy link
Collaborator

piever commented May 5, 2020

Now I understand the issue. I'm not 100% sure whether this was supported in the past. For now you can use

julia> struct PointX3f0
              data::NTuple{3, Float32}
       end

julia> tup = ([1f0], [1f0], [1f0])
(Float32[1.0], Float32[1.0], Float32[1.0])

julia> s = StructArray{PointX3f0}(data = StructArray(tup));

julia> s[1]
PointX3f0((1.0f0, 1.0f0, 1.0f0))

julia> s.data
1-element StructArray(::Array{Float32,1}, ::Array{Float32,1}, ::Array{Float32,1}) with eltype Tuple{Float32,Float32,Float32}:
 (1.0, 1.0, 1.0)

To make it easier, I guess in the various StructArray constructors I could also call StructArray on the arguments if they are not arrays already (but unfortunately that is not completely straightforward if data is not a (Named)Tuple), so I'm not sure if the that's worth it. With that, one could make the following work.

StructArray{PointX3f0}(data = ([1f0], [1f0], [1f0]))

For now it's probably best to just document the technique that works.

@SimonDanisch
Copy link
Member Author

Ah yeah, I hazily remember this recursive application of the StructArray constructor... That's probably why I remembered that it was supported :D

@cscherrer
Copy link

cscherrer commented Dec 4, 2020

I think this can be made to work. Instead of indexing into the values, we can index into a tuple or an array of lenses from Setfield.jl. Here's a little demo:

using BangBang
using Setfield

lenses(nt::NamedTuple) = _lenses(nt, ())

function _lenses(nt::NamedTuple, acc)
    result = ()
    for k in keys(nt)
        nt_k = getproperty(nt, k)
        # Add "breadcrumb" steps to the accumulator as we descend into the tree
        acc_k = push!!(acc, Setfield.PropertyLens{k}())
        ℓ = _lenses(nt_k, acc_k)
        result = append!!(result, ℓ)
    end
    return result
end

# When we reach a leaf node (an array), compose the steps to get a lens
function _lenses(a::AbstractArray, acc)
    return (Setfield.compose(acc...),)
end

Then, e.g.,

julia> nt = (a=(b=[1,2],c=(d=[3,4],e=[5,6])),f=[7,8]);

julia> lenses(nt)
((@lens _.a.b), (@lens _.a.c.d), (@lens _.a.c.e), (@lens _.f))

julia> [get(nt, ℓ) forin lenses(nt)]
4-element Array{Array{Int64,1},1}:
 [1, 2]
 [3, 4]
 [5, 6]
 [7, 8]

I need this kind of nested structure for samples in Soss.jl, so I'm keen to get it working :)

@cscherrer
Copy link

@piever I'm digging into this approach, hope to get to a PR before long. Any obvious difficulties you would expect? Pinging @tkf too, since you're a contributor here and a lens wizard :)

This was referenced Dec 4, 2020
@piever piever closed this as completed Dec 10, 2020
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

No branches or pull requests

3 participants