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 convenient functions stack/unstack #26

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

johnnychen94
Copy link
Contributor

@johnnychen94 johnnychen94 commented Aug 24, 2021

Unlike Align/Slices, the input to stack/unstack is the outer dimensions. It's possible to support multiple dimensions, but I didn't bother here because my direct target is one-dimensional case.

@DhairyaLGandhi this is a direct replacement for Flux.stack/Flux.unstack without allocating memories.

@DhairyaLGandhi
Copy link

How does it behave with AD?

@johnnychen94
Copy link
Contributor Author

Good question. Without even trying it, I bet it doesn't work here. I was just thinking about the data preparation stage stack(imgs, 4) when I'm writing the image data layout blog for Flux.

This package is pretty lightweight so I believe @bramtayl won't want to have extra dependencies here to support AD.

@DhairyaLGandhi
Copy link

DhairyaLGandhi commented Aug 24, 2021

Right, I tend to agree. I know that for LazyArrays, we had to support the apply! function which was the simple API that all of LazyArrays exposed. If JuliennedArrays has a similar format, we can add that somewhere too to keep the package light and AD ready.

@DhairyaLGandhi
Copy link

To be clear, it might still work (LazyArrays did at the time) but the adjoint meant that it remained lazy in the backwards pass as well, so it was needed for efficiency.

@johnnychen94
Copy link
Contributor Author

I've opened another PR #28 for test updates and enhancements, thus 1b607d8 is unnecessary now.

@bramtayl
Copy link
Owner

bramtayl commented Aug 25, 2021

Hi! I'm glad you find the package useful. I'd be very happy to accept a PR for auto-differentiation (although I'm not sure I could understand it enough to review it). I'm not sure I understand why stack/unstack are needed though?

@bramtayl
Copy link
Owner

Also, on the AD part, have you seen this? https://github.com/mcabbott/SliceMap.jl

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

Successfully merging this pull request may close these issues.

3 participants