Change DEMs from mask to image (is_image=True)

[dmeaux]

The documentation for the datasets.RasterDataset.is_image property is vague and has not clearly stated what one should do when it comes to a field data model (raster surface data). In the context of Computer Vision and Deep Learning, raster surfaces should be considered "images" and not "masks" because they are not a segmentation mask where pixels have been classified but are instead a range of values akin to DNs in an image.

In the case of raster surfaces, like DEMs, Digital Surface Models (DSMs), Digital Terrain Models (DTMs), or even another raster surface, such as temperature, atmospheric pressure, soil moisture content, etc. the is_image property should be set to True and treated like an image when the class inherits from datasets.RasterDataset.

The ASTER and EU Digital Elevation Model datasets have previously been set as masks instead of images when they are not segmentation masks but raster surfaces. This pull request clarifies the documentation for RasterDataset.is_image and rectifies the Aster Global DEM and EU-DEM datasets.

[dmeaux]

@microsoft-github-policy-service agree

[adamjstewart]

I personally agree with this distinction. DEMs and weather data are more likely to be used as inputs to a model than as outputs.

[adamjstewart]

What is the purpose of this change?

[dmeaux]

I was just trying to get it to pass validation.

[adamjstewart]

It should pass fine in CI without modification. It may not pass locally if you don't have pyvista installed.

[dmeaux]

Got it. I'll revert the change and install pyvista locally.

[dmeaux]

For what it's worth, I still get this error from mypy when I run the suggested validation checks: torchgeo/datasets/vhr10.py:476: error: Unused "type: ignore" comment [unused-ignore]

[adamjstewart]

That suggests you still don't have pyvista installed in the same environment where mypy is installed

[dmeaux]

In fact, this is blocking my commit.
mypy.....................................................................Failed

• hook id: mypy
• exit code: 1

torchgeo/datasets/vhr10.py:476: error: Unused "type: ignore" comment [unused-ignore]
torchgeo/datasets/vhr10.py:528: error: Unused "type: ignore" comment [unused-ignore]
Found 2 errors in 1 file (checked 292 source files)

[adamjstewart]

This is why I don't use precommit 😄

[adamjstewart]

I think we can fix this by adding pyvista to precommit

[adamjstewart]

Fixed by #1781. You can undo this change now.

[isaaccorley]

There's an entire area of research for height estimation of a DSM from an image. Weather data is also commonly used as a target. I think this may be too dependent on the user. There's nothing stopping anyone from subclassing the base dataset and changing is_image=True.

[isaaccorley]

I think the underlying issue is that we are using "mask" instead of "target" as the dict keys. So maybe we should be modifying the attribute to be is_target instead of is_image

[calebrob6]

So maybe we should be modifying the attribute to be is_target instead of is_image

I like this idea better (e.g. in autoencoders you might even want an image to be a target).

[dmeaux]

I like the idea of is_target as well. However, I would like to draw attention to the RasterDataset.dtype property, which "overrides the dtype of the data file via a cast" depending on the is_image property. I personally have cases where the target values should be a float and not a long . Could this functionality be rethought as well?

[isaaccorley]

I think we actually had a an open issue (maybe even a PR?) to fix the forcing of the dtype based on the is_image attr but I'll have to do dig for it.

[dmeaux]

I'm willing to look for it and do whatever fixes are decided upon, if y'all are cool with it.

[dmeaux]

I found PR #1149 that references the dtype behavior.

As far as I can tell from reading the various linked PRs, is_image was created along with dtype to make sure that all channels of an input dataset were cast to torch.float32 in order to work with torch.nn.Conv2D. Looking through kornia and pytorch, I don't see any reason why the target/segmentation mask needs to be cast to a torch.long. I only see why the image/input needs to be a torch.float32. (Someone please correct me if I missed something.)

So, I would suggest:

• change is_image to is_input
• change dtype from

  if self.is_image:
          return torch.float32
      else:
          return torch.long

  to

  if self.is_input:
          return torch.float32

• implement the changes I previously suggested for the DEM type datasets (changing is_image to is_input)
• refactor the existing datasets that use is_image to use is_input
• update the RasterDataset documentation
  • "image" becomes "input data", and specify that it can be single or multi channel raster data (float or integer)
  • "mask" becomes "target" and specify that target is the target data, which could include a single or multi channel segmentation mask or another type of raster layer, including one with float values.
  • dtype property: specifically state that it converts input data to torch.float32 for compatibility with pytorch.nn.Conv2D

[adamjstewart]

I would prefer to keep the naming scheme matching with Kornia for now, at least until we decide to switch from Kornia to Torchvision transforms v2. We actually went out of our way to call it image instead of input for the reasons mentioned in kornia/kornia#2193. Also see #985 for further discussion on this.

[adamjstewart]

I would like to draw attention to the RasterDataset.dtype property, which "overrides the dtype of the data file via a cast" depending on the is_image property. I personally have cases where the target values should be a float and not a long . Could this functionality be rethought as well?

This is simply the default behavior. If you would like to change this in your dataset, you can add:

dtype = torch.float32

[adamjstewart]

Going back through old PRs now. It seems like we still haven't reached a consensus on this PR. Is the original motivation for this PR related to the default dtype behavior (which is easy to override) or the name itself (which might be confusing)? Or is there some fundamental reason why this change is necessary?

[dmeaux]

To me, the issue is more in terms of documentation. I would just like to see more clarification about what is the purpose of this property and its relationship to the dtype property.

The real answer to what needs to be done lies in the answer to, "What is the purpose of is_image?". When someone goes to create a Dataset for something other than an image, such as a DEM, should they set it to False and then override the dtype property if the dataset has non-integer values?

What's the impact of setting is_image to True other than the default behavior of the dtype property? Does is_image just denote that it's an image versus a surface, does it denote that it's the source versus the target, or does it mean something else? I think that if it just denotes that it's an image, then it's fine as is, but documentation should be added to dtype that says if you're creating a new Dataset for a surface model that has non-integer values, be sure to override the dtype property.

Why does the default behavior of dtype depend on this attribute? I think this should be noted in the documentation too.

While the EU DEM v1.1 is no longer available, I have a copy of the entire dataset. It's natively Float32 GeoTIFFs, not integers, yet the EUDEM Dataset in TorchGeo doesn't override the dtype property to set it to Float32. I think something needs to be changed so that the values are not converted to integers.

The ASTER DEM data is still available for download, and the TorchGeo Dataset is fine how it is, since it's delivered as 16-bit signed integers.

Does this make it any clearer?

BTW, FWIW, next time, I'll start this off as an issue instead of just doing a pull request.

[dmeaux]

Oh, and I'm willing to make the changes, whatever you guys decide as the maintainers.

[adamjstewart]

Thanks for the clarification. Let me try to clarify how I envision the is_image and dtype properties. Feel free to copy/modify these descriptions and update the docs/tutorials. We may or may not also change names/values, but let's decide that after clarification.

is_image

Controls two different things.

The first is the name of the key that we use in the sample returned by __getitem__. True uses "image", False uses "mask". The goal here is to use the same names as Kornia uses, so we try to avoid inventing a new "dem" key, for example. Ideally, these should be as different as possible, since the user may want to combine multiple datasets with different keys. If the same key is used for multiple datasets (2 "image" and 1 "mask"), the channels will be stacked so that there's still a single value for that key.

The second is to change the default value of dtype. See below.

dtype

Defaults to float32 for is_image = True and long for is_image = False. This is what we want for 99% of datasets, but can be overridden for pixel-wise regression masks or integer images. Uint16 and uint32 are automatically cast to int32 and int64 (respectively) because numpy supports the former but torch does not.

Kornia and torchmetrics are picky about dtypes, and unfortunately disagree about these. If I recall (correct me if I'm wrong), Kornia augmentations require ALL inputs to be floats, while torchmetrics metrics require all inputs to be long. We have some hacky code scattered about to handle this so that users don't need to think about this.

[dmeaux]

Thanks! I'll make the changes to reflect that and will let you know when they're done.

[dmeaux]

I've updated the code to reflect what you wrote, @adamjstewart. I pretty much used what you wrote but tweaked the wording a little bit for different contexts. The asterdem dataset's is_image property is now set to True and the dtype property is overridden to return torch.long. The eudem dataset's is_image property is now also set to True.

[adamjstewart]

I wonder if we can make this more succinct like the other attributes.

[dmeaux]

Does this work?

[adamjstewart]

Why is one DEM long and the other is float32? To further clarify, dtype is the data type we cast to for kornia/torchmetrics, not the data type of the original files.

P.S. Did you mean to close this PR?

[dmeaux]

OK, I understand what you're saying now, and I've mad both DEMs float32.

I am, however, a little confused about what case you meant when you say "integer images" in the following:

Defaults to float32 for is_image = True and long for is_image = False. This is what we want for 99% of datasets, but can be overridden for pixel-wise regression masks or integer images. Uint16 and uint32 are automatically cast to int32 and int64 (respectively) because numpy supports the former but torch does not.

I just want to make sure the documentation is as clear as possible, and I think what I have for dtype in custom_raster_dataset.ipynb in the tutorial is unclear, incorrect, or misleading.

[adamjstewart]

I guess I can't really think of an application that requires integer images as input. Let's just remove that part of the sentence.

[dmeaux]

Ok, thanks for the quick response. I've removed that part of the sentence and committed the changes.

[adamjstewart]

I think most of the documentation changes are uncontroversial. The change from mask to image is more controversial, and may be unwanted for many users. I suggest splitting this into separate PRs so we can merge the first sooner and the latter later.

[dmeaux]

I changed the documentation table to "DEM". I'll separate the documentation changes to a new, separate PR.

[dmeaux]

The documentation changes have been submitted as PR #1811.