-
Notifications
You must be signed in to change notification settings - Fork 411
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
Datamodule augmentation defaults #1841
Comments
For the record, torchvision also divides by 255 by default. But yeah, I'm fine with dividing by 1 by default and allowing data modules to override the std dev. |
They only divide by 255 if it is uint8, which makes more sense (but still trips me up sometimes). |
I'm fine with changing the default to 1, just need to add a backwards incompatible tag to warn people. And convert all existing data modules to 255 (assuming that this is right). Want to make a PR? Note that some of @lcoandrade's confusion in #1822 was that Raster Vision divides by 255 automatically but he wasn't sure if TorchGeo did too. Maybe he has opinions on this. |
Hi there! I think the default behavior should be normalize to [0, 1] according to the data type used. This would avoid confusion. I also think this information should be clear in the API. I could only check the normalization behavior checking the code on GitHub after @adamjstewart mentioned it to me. |
Just my two cents here: "Automatic" normalizing data is a really painfully experience when working of mostly of raw satellite data. A simple example, is the sentinel-2, where we have int32 data from the L2A data, but it shouldn't be divided by 65536 to be normalized, but through dividing the data by 10k, then clipping values to be [0-1]. One thing that might help is defaults to the image inputs expected be always 0-1, or maybe allow the user entry within a callable when initializing it to just handle the data -- this is something I always need to hack through to be able to use the libraries |
I can do a PR with per-channel min-max scaling to [0,1] as the default augmentation. I'm actually using this type of scaling for my own work but my implementation requires the user to have the global mins and maxes specified somewhere, as is currently the case with mean and standard deviation. I think you should be responsible for your own data and knowing it's value range is not that harsh of a requirement in my opinion, so let me know if this OK and I can proceed with it. |
This isn't a good idea. We don't want batch-wise normalization, we want a single normalization parameter for the entire dataset. Let's default to |
Ah, please excuse the confusion; by "per-channel" I meant that if you have an This approach was required in my case because half my bands are aerial unsigned 8-bit imagery whereas the remaining ones represent rasterised LiDAR attributes (e.g., reflectance, slope, etc.), each with its own unit of measurement and value range. But it’s true that all batches are treated the same way and are scaled according to global parameters, like you mention. That being said, I think doing nothing may be a more general approach. |
Summary
Currently the datamodules divide by 255 by default. This can confuse users who would expect no augmentations by default. We should make this WARNING level clear or not divide by default.
Rationale
No response
Implementation
No response
Alternatives
No response
Additional information
No response
The text was updated successfully, but these errors were encountered: