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

Add an inner StackLayers <: AbstractStackLayers object #473

Open
rafaqz opened this issue Mar 6, 2023 · 1 comment
Open

Add an inner StackLayers <: AbstractStackLayers object #473

rafaqz opened this issue Mar 6, 2023 · 1 comment

Comments

@rafaqz
Copy link
Owner

rafaqz commented Mar 6, 2023

A few things have led me to this possible refactor. An inner wrapper for stack layers:

struct StackLayers{K,T<:Tuple,D<:Tuple,LD<:Union{Tuple,Nothing},LM<:Union{Tuple,Nothing}}
    data::T
    dims::D
    layerdims::LD
    layermetadata::LM
end

#457 wants alternates storage mechanisms, and #471 is trying to fix performance of stacks.

Most performance problems are now in how the stack is rebuilt after map. The remaining fixes for that will involve simplifying the data, layerdims andf layermetadata fields.

  • all should be Tuple rather than NamedTuple to simplify the type.
  • keys are in type parameter K
  • layermetadata can often just be nothing
  • layerdims can be nothing when all layers have the same dims
  • combinedims should only happen when it absolutely has to, as it's slow once you have a lot of layers.
  • rebuild_from_arrays can be very simple and fast in the case where all layers have the same dims.

This could all be handled in a StackLayers object. Meaning it could also be used inside other wrappers like RasterStack with very little work, protecting packages that extend AbstractDimStack from dealing with these internals and performance considerations.

Other benefits are:

  • Rasters.FileStack could be another <: AbstractStackLayers
  • istable on types not instances #475 can be solved with a MStackLayers object that new layers can be pushed to.
  • we can easily switch between mutable/immutable states by defining conversion between StackLayers and MStackLayers, with no changes required in to the AbstractDimStack wrapper.
  • We can define setproperty!(stack, ::Symbol) and push!(::AbstractDimStack, ::AbstractDimArray) and forward them to the StackLayers object, getting an error for immutable objects, and updates for MStackLayers.
mutable struct MStackLayers
    keys::Vector{Symbol}
    data::Vector{<:AbstractArray}
    dims::Tuple
    layerdims::Vector
    layermetadata::Vector
end

@sethaxen any thoughts on this?

@rafaqz
Copy link
Owner Author

rafaqz commented Nov 2, 2024

This actually just kind-of works as is now, as AbstractDimStack has K as the first parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant