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 Base.parent work on TrackedArrays #35

Open
oxinabox opened this issue Jun 12, 2019 · 4 comments
Open

Make Base.parent work on TrackedArrays #35

oxinabox opened this issue Jun 12, 2019 · 4 comments

Comments

@oxinabox
Copy link
Member

Most wrapper arrays define Base.parent to get the inner array.

For consistency it would be nice if TrackedArray did also.

For most used Tracker.data is what you want, because you want something that does identity to nontracked arrays.

But for a few things you do actually want to access the parent

@MikeInnes
Copy link
Member

As in, parent(x::TrackedArray) = data(x)?

Problem is that it conflicts with the normal meaning parent when x is an Adjoint (that happens to be tracked). e.g. if I write f(x) = parent(x'), this should behave like the identity function whether x is tracked or not.

@oxinabox
Copy link
Member Author

As in, parent(x::TrackedArray) = data(x)?

yes.

Problem is that it conflicts with the normal meaning parent when x is an Adjoint

Is that really the normal meaning of parent?
That doesn't hold for other types defining parent.
E.g. SubArray

julia> parent(@view((x')[:,:])) |> typeof
LinearAlgebra.Adjoint{Int64,Array{Int64,1}}

@MikeInnes
Copy link
Member

It does hold in the sense that if you pass a SubArray to that f it still behaves as the identity. I can use parent in generic code as long as it reliably strips away the most recent wrapper, but TrackedArray violates this assumption by forcing itself to always be outer, regardless of what order tracking and views were applied in.

We could do this if we reliably treated tracking as if it were a view (so that you could have adjoint-of-tracked, view-of-tracked etc.) but that has its own problems w.r.t. dispatch of matmul and so on, so it's not the route we've taken.

@oxinabox
Copy link
Member Author

Context:
invenia/NamedDims.jl#20

This can be revisited later, when and if that idea firms up.

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

2 participants