[DRAFT] FLAIR#2 Dataset and Datamodule Integration #2394
Conversation
…mg and msk) Updates in the custom raster dataset tutorial and the actual file documentation. The previous recommended approach (overriding `__get_item__`) is outdated. Refs: microsoft#2292 (reply in thread)
Co-authored-by: Adam J. Stewart <[email protected]>
Co-authored-by: Adam J. Stewart <[email protected]>
… type individually
Not fully functioning yet, contains copy paste from other datasets
Additionally, some small code refactors are done
…d refine plotting
Using the entire sentinel-2 image and a matplotlib patch to debug, otherwise it is really hard to find correct spot due to low resolution
…y()` for sentinel With the nested dict, it was not possible to download dynamically
md5s might change due to timestamps, this eases the process of changing md5
|
Thanks a lot for the PR, here are just some answers at the top of my head, others might have additional opinions.
Not sure about this one, but your proposition seems reasonable and as long as this is clearly documented, I would think it is fine.
I think that depends on how, the large sentinel tile is expected to be used/useful for a model. The very low resolution area does not seem useful intuitively. I think having the flag you implemented is nice. If you provide the entire tile, there will have to be a collate or cropping function in the datamodule, otherwise the dataloader cannot return consistent batches, as by default it tries to batch individual samples into a tensor by dict key.
I actually like that approach.
Classes should definitely be ordinally mapped to start from 0, otherwise there will be issues with the |
| @@ -1,62 +1,64 @@ | |||
| Dataset,Task,Source,License,# Samples,# Classes,Size (px),Resolution (m),Bands | |||
There was a problem hiding this comment.
something seems off here, since the entire csv file changed, and not just the FLAIR2 addition.
There was a problem hiding this comment.
Probably line endings
There was a problem hiding this comment.
Should I try to change to the original line endings, or does it not really matter?
There was a problem hiding this comment.
Change to the original line endings please, that will make git diff and git blame much more useful
tests/data/flair2/data.py
Outdated
| from pyproj import CRS | ||
|
|
||
| # General hyperparams | ||
| IMG_SIZE = 512 |
There was a problem hiding this comment.
For the toy data we usually use smaller image sizes like 32 just to speed up tests.
| """ | ||
| super().__init__(FLAIR2, batch_size, num_workers, **kwargs) | ||
|
|
||
| self.patch_size = _to_tuple(patch_size) |
There was a problem hiding this comment.
The definition of patch_size is useful if there is an additional kornia Resize augmentation under self.aug, however, we would also have to consider how this plays with the larger sentinel tile. I am still not sure what purpose the additional Sentinel 2 should provide in terms of model performance if the high res image only covers such a tiny area.
There was a problem hiding this comment.
I am by no means an expert, but have been reading some papers in the domain the last year. I suppose, the increased feature dimensions provided by the hyperspectral bands of sentinel-2 have shown to increase model performance.
Furthermore, in the datapaper [p. 6] of FLAIR2 the authors argue:
The interest of the extended spatial information provided by the Sentinel-2 super-patches is particularly visible in the last two rows of Figure 4. Indeed, the location on a beach or on a lake is difficult to determine from the aerial image alone, and could easily be confused with the sea for example in the last row.
The model leveraging the additional information outperform a standard baseline model:
| Model | mIoU | Building | Pervious Surface | Impervious Surface | Bare Soil | Water | Coniferous | Deciduous | Brushwood | Vineyard | Herbaceous Vegetation | Agricultural Land | Plowed Land |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| U-Net | 0.547 | 0.8009 | 0.4727 | 0.6988 | 0.3076 | 0.7985 | 0.5758 | 0.7014 | 0.2392 | 0.6012 | 0.4653 | 0.5449 | 0.3583 |
| U-T&T | 0.5594 | 0.8285 | 0.4980 | 0.7204 | 0.2982 | 0.8009 | 0.6041 | 0.7189 | 0.2541 | 0.6580 | 0.4684 | 0.5478 | 0.3157 |
| U-T&T best | 0.5758 | 0.8368 | 0.4995 | 0.7446 | 0.3959 | 0.7952 | 0.6339 | 0.7239 | 0.2485 | 0.6678 | 0.4750 | 0.5513 | 0.3381 |
There was a problem hiding this comment.
Thanks for pointing that out, I should have looked at the paper myself... In that case, if we want to support a resize, it would be more involved, if we wont to resize the sentinel image as well but keeping the resolution difference. Or still just provide the Sentinel 2 scene as context.
There was a problem hiding this comment.
My suggestion would be to initialize a sentinel scaling of 0 to 1 in the model. This in turn could be injected into the loading function.
- 0 would resize the image to the exact extent of the aerial images.
- 1 would take the entire image
- anything between would linearly scale from aerial to original extent
This way, both a straight-forward streamlined model (i.e. uses sentinel as aerial extent as just another source of information) and more sophisticated models that fuse classification at a later point in time could be used. Any suggestions for improvement?
torchgeo/datamodules/flair2.py
Outdated
| num_workers: Number of workers for parallel data loading. | ||
| augs: Optional augmentations to apply to the dataset. | ||
| **kwargs: Additional keyword arguments passed to | ||
| :class:`~torchgeo.datasets.Potsdam2D`. |
| augs: Optional augmentations to apply to the dataset. | ||
| **kwargs: Additional keyword arguments passed to | ||
| :class:`~torchgeo.datasets.Potsdam2D`. | ||
| """ |
torchgeo/datasets/flair2.py
Outdated
| return (tensor - tensor.min()) / (tensor.max() - tensor.min()) | ||
|
|
||
| # Define a colormap for the classes | ||
| cmap = ListedColormap([ |
There was a problem hiding this comment.
this can go above the init in class attributes
|
Hi! Sorry for the late response. First of all, thanks for the review. I will get to work on the proposed changes ;-) |
…; 256-256=>16x16)
…opposed to initializing with flag Previously, a lot of checks in all functions were necessary making it error prone. Like this, only a few functions have to be overridden.
|
Apart from the sentinel data, I think everything is on track now. Let me know whether you have a preferred way of me handling this. |
|
I was just testing the pull request, and there was an issue with the download, where it gets a 404 when trying to download Great work though! Awesome to see FLAIR being implemented 😃 |
| to_extract.append(dir_name) | ||
| continue | ||
|
|
||
| files_glob = os.path.join(downloaded_path, "**", self.globs[train_or_test]) |
There was a problem hiding this comment.
When instantiating the datamodule, it will extract the flair_sen_train zip-file on every run. I traced it here, and I believe it's becuase when going through when dir_name="flair_sen_train", I get files_glob="/home/jhj/dataset_test/FLAIR2/flair_sen_train/**/SEN2_*{0}.npy", with the curly braces at the end. This makes the glob.glob() fail in the next line. I tried manually setting it to files_glob="/home/jhj/dataset_test/FLAIR2/flair_sen_train/**/SEN2_*.npy", which fixed the issue.
There was a problem hiding this comment.
Seems like it's the same with flair_2_sen_test, which also gets the {0} in files_glob, and extracts the zip-file on every run.
There was a problem hiding this comment.
When instantiating the datamodule, it will extract the
flair_sen_trainzip-file on every run. I traced it here, and I believe it's becuase when going through whendir_name="flair_sen_train", I getfiles_glob="/home/jhj/dataset_test/FLAIR2/flair_sen_train/**/SEN2_*{0}.npy", with the curly braces at the end. This makes theglob.glob()fail in the next line. I tried manually setting it tofiles_glob="/home/jhj/dataset_test/FLAIR2/flair_sen_train/**/SEN2_*.npy", which fixed the issue.
I see why the unexpected behavior appears. For clarification: the format string is necessary, as there are two files inside the directory: mask and data files. To be able to respectively get only the corresponding mask or data file, I had to format the glob.
Would you be willing to share with me your code snippet so I can debug this real quick?
There was a problem hiding this comment.
I just manually removed the {0} in the files_glob string before running glob.glob():
files_glob = os.path.join(downloaded_path, "**", self.globs[train_or_test])
if "flair_sen_train" in files_glob:
files_glob="/home/jhj/dataset_test/FLAIR2/flair_sen_train/**/SEN2_*.npy"
if not glob.glob(files_glob, recursive=True):
to_extract.append(dir_name)
I.e., hard-coded files_glob so I could check if it stopped extracting the zip-file on every run. Not really a solution, but just wanted to see if I was the right place in the code.
There was a problem hiding this comment.
Just tested it again, and all download and data extraction works now 👍
|
Apologize if the comments were a bit scattered, and not really a proper review. I was just trying to run the code, and commented on the go with any issues I encountered 🙂 |
flair_2_centroids_sp_to_patch.zip vs. flair-2_centroids_sp_to_patch.json Refs: microsoft#2394 (comment)
Weird; I was sure I tried this. Thanks for letting me know. |
Instead of loading both sentinel-data/cloudsnowmasks into one sample, store them sperately. Same with the crop_indices.
Yeah, it seems like a weird error. Perhaps they changed it on their side, such that the zip-file naming was consistent. Although the json file inside still uses hyphen instead of underscore 🙂 |
torchgeo/datasets/flair2.py
Outdated
| files = [ | ||
| dict(image=image, sentinel=sentinel, mask=mask) | ||
| for image, sentinel, mask in zip(images, sentinels, masks) | ||
| ] |
There was a problem hiding this comment.
It seems like this part filters out most of the aerial images. I think it's because each Sentinel-2 image covers multiple aerial images. E.g., flair_aerial_train/D004_2021/Z1_NN contains 100 aerial images, and the corresponding flair_sen_train/D004_2021/Z1_NN contains one .npy file. So the association between Sentinel-2 and the aerial images should probably be done on a per-folder basis instead of per-file basis. E.g., something like:
sentinel_lookup = {"/".join(s["data"].split("/")[-4:-2]): s for s in sentinels}
files = [
dict(image=image,
sentinel=sentinel_lookup["/".join(image.split("/")[-4:-2])],
mask=mask)
for image, mask in zip(images, masks)
]
There was a problem hiding this comment.
Oh wow! What a severe mistake. Thanks for clarifying. Your approach seems very valid.
There was a problem hiding this comment.
That happens sometimes 🙂
I just tried training a model on the aerial data now with the code posted above, and all seems to be working. I haven't looked closely at the Sentinel-2 data, although they looked correct when I sampled a couple of elements in the files list in the debugger.
This bug caused to omit a good 90% of all images. Refs: microsoft#2394 (comment)
|
It got all the correct aerial imagery files now and the download and extraction is working. However, I'm getting a much more annoying bug now: I've had this issue before with Rasterio, and it was due to concurrency issues. This shouldn't really happen here, as the dataloader just iterates through a list of files, and each sample is a separate tif file. I can see there's also been a discussion here (#594), so perhaps @adamjstewart or @calebrob6 knows what's going on. My own guess is that it could be multiple workers having GDAL do simultaneous scans of the same directory when opening individual files in said directory. I.e., there's often many image files in each sub-folder in the dataset, and GDAL will scan the sub-folder for metadata when opening a file inside it, which could be the culprit. For now at least, I've added the code below in This has been added below the imports in the top: I've also minimized the time the datareader is open in Rasterio by closing it before doing the tensor operations. This shouldn't really solve the issue here, but added it anyways for good practice. In the and in the Not sure if it fixes it, but I'll train a handful of models and try it out. The issue coming from simultaneous directory scans is a bit of a guess. |
|
Never had this multiprocessing issue before, and I don't see anything in the code that looks sus. Can you reproduce this issue with the unit tests? If not I can also try downloading the dataset and reproducing locally. |
|
Unfortunately not. It pops up at random after training for quite a while. Encountered it the first time after having trained for almost 2 full epochs. I.e., it had already read the entire dataset once, and then got IReadBlock error during second epoch. There have been some reports with similar errors here: rasterio/rasterio#2053 . That's generally when reading the same file though, and I agree, nothing looks suspicious in the code. I actually thought my disk was broken, but then I remembered that that was my exact same thought last time I got this error with Rasterio. I'll try and let it run some more times and see if it keeps happening. Just saw in rasterio/rasterio#2053 (comment) that upgrading to newer version of GDAL might help. I'll also give that a try. |
FLAIR#2 dataset
The
FLAIR #2 <https://github.com/IGNF/FLAIR-2>dataset is an extensive dataset from the French National Institute of Geographical and Forest Information (IGN) that provides a unique and rich resource for large-scale geospatial analysis.The dataset is sampled countrywide and is composed of over 20 billion annotated pixels of very high resolution aerial imagery at 0.2 m spatial resolution, acquired over three years and different months (spatio-temporal domains).
The FLAIR2 dataset is a dataset for semantic segmentation of aerial images. It contains aerial images, sentinel-2 images and masks for 13 classes.
The dataset is split into a training and test set.
Implementation Details
NonGeoDataset,__init()__After discussions following #2303, we decided that at least until faulty mask data are fixed the flair2 ds will be of type
NonGeoDataset. Other than with commonNonGeoDatasets, FLAIR2 exposes ause_toyanduse_sentinelargument. Theuse_toy-flag will instead use the toy data which is a small subset of data. Theuse_sentinelargument on the other hand decides whether a sample includes the augmented sentinel data provided by the maintainers of FLAIR2._verify,_download,_extractAs each of the splits/sample-types (i.e.
[train, test], [aerial, sentinel, labels]are contained in a individual zip download, download and extraction has to happen multiple times. On the other hand, the toy dataset is contained in a singular zip. Furthermore, to map the super-patches of the sentinel data to the actual input image, aflair-2_centroids_sp_to_patch.jsonis required, which has to be equally has to be downloaded as an individual zip._load_image,_load_sentinel,_load_targetFor storage reasons, the elevation (5th band) of the image is stored as a uint. The original height thus is multiplied by 5. We decided to divide the height by 5 to get the original height, to make the trained model more usable for other data. See Questions please.
As mentioned previously, additional metadata has to be used to get from the$T \times C=10 \times W \times H$ vary both $T$ and $W$ , $H$ . This is problematic for the
sentinel.npyto the actual area. Initially for debugging reasons, we implemented to return not the cropped image but the original data and the cropping-slices (i.e. indices). Consequently, the images can be plot in a more meaningful matter. Otherwise, the resolution is so low that one can hardly recognize features. This was crucial for debugging to find the correct logic (classic y, x instead of x, y ordering mistake). We do not know if this is smart for "production code". See Questions please.Moreover, the dimensions of the sentinel data
datamodule. We have not done extensive research, but the varying dimensions seem to bug the module. Disabling theuse_sentinel-flag will make the module work.The labels include values from 1 to 19. The datapaper clearly mentions grouping classes$> 13$ into one class
otherdue to underrepresentation. We followed this suggestion. Furthermore, rescaling from 0 to 12 was applied. See Questions please.Questions
How shall we load/provide sentinel data? As cropped data or any other way. I do not see the current implementation as fit for production.
Shall we rescale the Classes to start from 0? Shall we group the classes as suggested in the datapaper?
Check integrity in
download_urldoes not seem to work (in unit-tests), why?check_integritycall otherwise it passes, even if md5s do not match.The github actions on the forked repo produce a magic ruff error (https://github.com/MathiasBaumgartinger/torchgeo/actions/runs/11687694109/job/32556175383#step:7:1265). Can you help me resolve this mystery?
TODOs/FIXMEs