Closed barucden closed 2 years ago
Funny coincidence then! :smile:
I wish we could move more operation implementation out of this package
ColorJitter goes against. Do you think it would be reasonable to move the operation to ImageContrastAdjustment.jl? If so, I can start an issue there and work on a PR.
Opening an issue and consider a generic implementation definitely have benefits and usually make the code more extensible. Maybe the question is, can we make ColorJitter
support more predefined operations in ImageContrastAdjustment
?
I don't play deep learning recently so maybe you get more idea of whether this kind of generalization is wanted.
At least for the current ColorJitter implementation, it's simple enough to be hold in this package.
IMHO, ColorJitter should do what it does now, as simple as it is. My reasoning was that it is a common image adjustment method (which can also be defined by histogram transformation) so maybe it would be a good fit for ImageContrastAdjustment.jl.
I think we should definitely support some of the operations defined in ImageContrastAdjustment.jl. I’ve seen a few of them being used in deep learning. But I guess they should be a separate operations, not ColorJitter
.
The specific struct
ColorJitterView
, which we use for lazy augmentation in ColorJitter, is just a MappedArray. (I did not know MappedArrays.jl when implementing the operation.)I don't think it is necessary to produce a new package version since nothing has changed, only the code is reduced.