-
Notifications
You must be signed in to change notification settings - Fork 46
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
Unify the augment interface #97
Comments
This is a long issue and I don't have enough bandwidth to actually discuss a potential solution for the moment. There are two points I'm concerned with:
I'm not very sure what my opinion on combining
I think the difference is: I also notice @lorenzoh is developing a new package DataAugmentation.jl so I'd like to invite him into the discussion. |
Thank you for your feedback!
I agree. Just note that the ambiguity comes from using the numeric arrays, not the colorant ones.
This is what we use now, right? I did not mention it in the first post but I also think this is a good approach.
Great summary. I feel like batch = [img1, img2, ...]
aug_batch = augment.(batch, pl)
# or
batch = cat(img1, img2, dims=3) # (H, W, N)
aug_batch = cat(augment.(obsview(batch, 3), pl), dims=3) I would be interested in @maxfreu's opinion too. |
Hey, author of DataAugmentation.jl here. I created DataAugmentation.jl to adress some of my pain points with Augmentor.jl, though that was a while ago and I don't know how much has changed since then. In DataAugmentation.jl
See the following links: DataAugmentation.jl doesn't have support for batches yet, but that will be implemented in the future as a wrapper around a semantic wrapper, i.e. instead of having a single image |
In my opinion it is absolutely ok and very clear to have two functions, one for single images, one for batches. Makes things easier programming-wise and it's also easy to communicate to users. @lorenzoh what exactly were your pain points? I think augmentor's design is flexible enough to be applied to various datatypes and support for masks is in the making, although keypoints etc are still missing. I wonder how augmentor's optimized, generated functions compare to DataAugmentation's use of buffers. I think the best thing would be to concentrate work on a single one-fits-all package (which in my opinion should be augmentor, because its fast). Also note that there is a third, small package: Augmentations.jl Now it gets lengthy and maybe a bit off-topic :D Just to give my use case: I work with satellite images, which can come with any number of bands, without a specific color connotation. When I load a single image from disk via ArchGDAL, it is usually an
Yes, that's maybe true, but sometimes it's also a nuisance that I can't just throw 3D arrays in. Albumentations for example expects CWH inputs, too - knowing this, I can follow the contract and that's it. Clearly not the most flexible solution, but it works.
So for the time being, in my opinion development should focus on 1 & 2, things like this issue can come later unless they are a blocker for other work. |
There are two things that
Just noticed the example you provided in #54 (comment) outbatch(X) = Array{Float32}(undef, (28, 28, 1, nobs(X)))
augmentbatch((X, y)) = augmentbatch!(outbatch(X), X, pl), y Every call of I didn't check it, but I'm wondering if |
Yes, you are right. I am sorry if the order that I used in the table confused anyone. Hopefully, it did not overshadow the main point of the proposal. I now realize I was maybe too eager to propose this. I did not have a clear vision how to proceed further, and now I see there are more steps to take before we can even discuss what's proposed in the original post. I took a quick look at DataAugmentation.jl, and I think we have some catching up to do in some aspects. @johnnychen94 Doesn't my example allocate memory for each batch too? In my work, batches are of different sizes quite often, so I tend to just allocate new batch each time. I'll check DataLoader. Thanks for the reference. |
tl;dr Remove
augmentbatch!
and haveaugment
andaugment!
support batch inputs. I think it's doable but there are some issues.Currently we have
augment(imgs, pl)
,augment!(outs, imgs, pl)
,augmentbatch!(out_batch, img_batch, pl)
,and we would be also interested in
augmentbatch(img_batch, pl)
.Should we drop the "batch" versions, and keep just
augment
andaugment!
? And if so, can we do it? I tend to think yes and yes.Why
It simplifies the interface. The user does not have to care if they work with batches or single images, the same function will work for both.
Having only a single function would help us solve #33 easily, too.
How
I believe the main issue is to tell if an input is a batch or not based on its type. Once we are done with that, we can just employ dispatching to call appropriate code.
In my limited view of the world, an image is a 3-dimensional array of shape
(H, W, C)
, whereH
,W
,C
is the height, width, and number of channels. WhenC=1
, the image can as well be a 2-dimensional array of shape(H, W)
.A batch is a 4-dimensional array of shape
(N, H, W, C)
, whereN
is the number of images. Again, whenC==1
, the batch can be a 3-dimensional array of shape(N, H, W)
.In Julia, the situation is a little bit more complicated, as the array elements tend to be structure instances (like
RGB
) and the "channel" dimension is omitted.Also, the dimensions can be permuted (e.g.,
(H, W, C, N)
), but Augmentor already deals with that to some extent.The following table maps the input types to the decision batch/image. Unfortunatelly, there is one ambiguity (typed in bold).
(N, H, W, C)
(N, H, W)
(N, H, W)
(N, H, W)
(H, W, C)
(H, W)
(N, H, W)
(H, W)
I came up with two ways to resolve the ambiguity, both of which are sort-of bad:
I could see this ambiguity take down the whole proposal. Yet, I would love if we could figure it out.
Mixing batches and images
Now, it is possible to do
augment((img1, img2), pl)
which applies the same operations to both
img1
andimg2
. On the contrary, thisaugmentbatch(batch, pl)
applies possibly different operations to different images in
batch
(assuming the non-mutating version ofaugmentbatch!
exists). Consequently, the following would not be well-defined:augment((img1, img2, batch), pl)
There is a contradiction in it: "apply the same operations to
img1
,img2
, andbatch
, and also apply different operations to images inbatch
".For this reason, I think we should disallow mixing batches and images on the input.
Dealing with semantic wrappers
We would require the following to work:
Questions for discussion
I am interested in any opinion of yours, but here I list two questions to start a discussion:
The text was updated successfully, but these errors were encountered: