search

FluxML / DataAugmentation.jl

Flexible data augmentation library for machine and deep learning
https://fluxml.ai/DataAugmentation.jl/dev/
MIT License
41 stars 17 forks source link

Add `PermuteDims` transformation #89

Closed adrhill closed 5 months ago

adrhill commented 6 months ago

Adds PermuteDims, a thin wrapper around permutedims for use on ArrayItems.

This should help with Flux compatibility (https://github.com/FluxML/DataAugmentation.jl/issues/56): HWC to WHC conversions can be implemented by adding PermuteDims(2, 1, 3) to a pre-processing pipeline.

darsnack commented 6 months ago

ImageToTensor already does permute the dimensions to get the channel last. We should change that so that it also permutes width/height. We can still add this transformation, because it could be useful in other contexts.

Why not use PermuteDimsArray directly to avoid copying? You can see how ImageToTensor does it as reference.

adrhill commented 6 months ago

Why not use PermuteDimsArray directly to avoid copying?

Good point, addressed in 3e44d1d.

ImageToTensor already does permute the dimensions to get the channel last. We should change that so that it also permutes width/height.

Wouldn't this be considered breaking? We could introduce more explicit ImageToWHCTensor and ImageToHWCTensor transformations and deprecate ImageToTensor.

darsnack commented 5 months ago

Wouldn't this be considered breaking? We could introduce more explicit ImageToWHCTensor and ImageToHWCTensor transformations and deprecate ImageToTensor.

True, but do we really need the explicit versions if we have PermuteDims? For now, why don't we throw a deprecation warning in ImageToTensor with instructions how to get HWC in the future using (fixed) ImageToTensor + PermuteDims?

I think it's more friendly to not have the user worrying HW or WH in the long term API. An alternative would be to have explicit forms and let ImageToTensor default to one.

adrhill commented 5 months ago

Sounds good. I will open a separate PR for changes to ImageToTensor. I think this one is ready to review/merge.

darsnack commented 5 months ago

Sounds good, thanks!