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

Discussion on padding in only 1 or defined dimensions,and adding dims setting as a keyword #24

Open
ashwani-rathee opened this issue Nov 25, 2020 · 4 comments

Comments

@ashwani-rathee
Copy link

From #164,Padding in only 1 direction make sense for case where we don't want padding in 2 dims and output of same size is not needed/required.ImagePyramids being a good case for this.

using paddedviews:

using mosaicview:


JuliaImages/juliaimages.github.io#164 (comment)
@johnnychen94 's comment serves as the starting point for this discussion

The current mosaicview is built based on paddedviews and sym_paddedviews, whose outputs are of the same size. For the purpose of pyramid visualization, it makes sense to visualize as your option 1 does.

Instead of providing some dirty for loop in the demo, I think a better way is to improve how mosaicview works. Here's what I have in mind:

add a dims keyword to paddedviews and sym_paddedviews to specify we only want dims dimensions to be padded.
then also add dims keyword to mosaicviews.

The dims keyword should follows the same options as cats do, that all of dims=1, dims=(1, 2) and dims=1:2 are valid values. To be backward-compatible, we need to make the default option dims=1:ndims(first(As)).

mosaicview(pyramid; nrow=1, center=false, dims=1) is what I have in mind; that we only pad along the height dimension.

@johnnychen94
Copy link
Member

We could first add dims keyword to paddedviews and sym_paddedviews in PaddedViews.jl. How's your progress on it?

@ashwani-rathee
Copy link
Author

I haven't been able to figure it out yet,so I would say I am going pretty slow here.

@johnnychen94
Copy link
Member

With JuliaArrays/PaddedViews.jl#49 merged, I think we can now work on this issue.

Just need to clean out a way to pass dims keyword from mosaic/mosaicviews to paddedviews/sym_paddedviews.

I think this method can now be safely deleted:

# compat: some packages uses this method
_padded_cat(imgs; center::Bool, fillvalue, dims) = _padded_cat(imgs, center, fillvalue, dims)

and previously used dims can be renamed to stack_dims.

@johnnychen94
Copy link
Member

This turns out to be impossible unless we redesign mosaic for array of array inputs. The current pipeline is:

  1. padding arrays so that they have common axes
  2. stacking padded arrays (via StackArray) into a higher dimensional array
  3. unstacking the arrays onto 2D array (via MosaicView)

Step 2 requires arrays to be common axes.

For array of array inputs, the more efficient and also more flexible way is to create a lazy cat view.

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