Preserve metadata on getindex operation when scalarizing

[contradict]

No description provided.

[ChrisRackauckas]

@shashi @YingboMa should the metadata be the same, or should it know that it's derived from indexing this object somehow?

[shashi]

It should not be the same... And I think this should not be added... I would say this is a usecase where tagged-data like API becomes useful. You should be able to define something like:

@metadata getindex(::MetadataType, idx) = compute_output_metadata_here

And it should be handled for every key which registers in this way.

I'd be interested in adding this general feature, but I'm looking for more substantial examples where this is super useful.

Until then, I think you should just try to propagate the metadata outside of the package's behavior (by walking the tree once constructed). The method in this PR seems not correct to me.

[contradict]

The present behavior seems surprising to me. Is the correct resolution to document that scalarize does not preserve metadata or to create an issue to record this and come back to it when a more complete solution can be designed?

[ChrisRackauckas]

I don't think the approach is generally wrong, it's just that A shouldn't preserve the same exact metadata as A[i]. Something should happen when indexing since it's not the same variable.

[contradict]

The new thing that happens here is not the indexing, this was an indexing operation coming in and is still one on return (possibly even exactly the same one if there were no array operations in the variable or indexing arguments). The new thing is that scalarize has been applied to the variable and to the indexing arguments. In the other branches, metadata is preserved with no modifications to indicate scalarization, what is special about scalarization of indexing?

[ChrisRackauckas]

What if you just add to the metadata the index?

[contradict]

In the test case I have the index is just an integer. What is the right thing to wrap that in so I can attach metadata?

[ChrisRackauckas]

@shashi have a preference?

[shashi]

We already do something like this for @variables x(t)[1:20] I believe, the hook

Symbolics.jl/src/array-lib.jl

Line 6 in 43f93d0

@wrapped function getindex_posthook(f, x::AbstractArray)

So if you just do getindex_posthook(f, x) then f will be called on every indexing operation on x and the arguments will be, the result of the getindex operation, and the indices passed in. At this point you can choose to attach some sub-metadata.

And this function

Symbolics.jl/src/variable.jl

Line 21 in 43f93d0

function recurse_and_apply(f, x)

will make it sticky: it will make it work on the subarrays as well. But I recommend writing your own because that's untested and dead code. 🗡️

[shashi]

In general, it would be nice to have something that makes sense for all purposes and is extensible. I'm also ok with attaching the index to the metadata, you could just create a new type called MetadataView(metadata_item, index), but metadata is required to be a ImmutableDict, so you'd have to create a view for each key that's already in the array's metadata. Maybe it makes sense to define a generic metadata_view(::MetadataKeyType, array, idx...) which providers of MetadataKeyType can overload to get the immediate behavior they want. In all other cases, it could just fallback to constructing MetadataView