Skip to content

Conversation

@griembauer
Copy link
Contributor

This PR fixes the following issue in i.zero2null:

Bug description

In some cases, i.zero2null results in blurred (=extrapolated) image borders where null() should be. This occurs if there already is a nodata-border in the input map. An example of a Sentinel-2 scene processed with i.zero2null:
bestmap_izero2null_bug

to reproduce

  • start GRASS in an epsg:32630 location
  • download Sentinel-2 scene S2A_MSIL2A_20210329T105631_N0214_R094_T31UCV_20210329T134554 (e.g. via i.sentinel.download)
  • set the region to g.region n=5944230 s=5603910 w=564260 e=823750 res=10 -p
  • import a Band from the scene with i.sentinel.import extent=region ...
  • run e.g. i.zero2null map=T31UCV_20210329T105631_B08_10m

fix

The bug is introduced because
a) Setting the region to the input raster does not necessarily set it to the maximum no-null() extent. This is now ensured with g.region zoom=...
b) After setting the region to the input raster, there may be border areas which are already null(). These are now excluded from the result of r.grow.distance too.

@griembauer griembauer requested a review from metzm April 7, 2021 12:31
@griembauer griembauer added the bug Something isn't working label Apr 9, 2021
@neteler
Copy link
Member

neteler commented May 6, 2021

May this PR be merged?

Copy link
Contributor

@metzm metzm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I did not encounter this case in my tests, please merge!

Copy link
Contributor

@metzm metzm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, the fix in this PR is not correct. i.zero2null must be used before reprojection to avoid artefacts at the transitions from zero to non-zero cells. In this example, i.zero2null is used after reprojection (from UTM zone 31 to UTM zone 30), this is too late, artefacts have already been introduced.
A proper fix would be to check if there are both NULL and zero cells in the input image (with e.g. r.info -s, if yes, exit with a warning like "Sorry, too late, the input image has already been reprojected".

@metzm
Copy link
Contributor

metzm commented May 14, 2021

A good place for this zero2null method would be r.import with a new -z flag where zero2null could be applied before any reprojection happens. I would keep this i.zero2null addon anyway for cases when r.in.gdal is used directly.
@griembauer Could you create a corresponding PR for r.import?

@ninsbl
Copy link
Member

ninsbl commented May 14, 2021

A good place for this zero2null method would be r.import with a new -z flag where zero2null could be applied before any reprojection happens. I would keep this i.zero2null addon anyway for cases when r.in.gdal is used directly.
@griembauer Could you create a corresponding PR for r.import?

See also:
OSGeo/grass#580

@griembauer
Copy link
Contributor Author

Thanks @metzm, I now understand the correct processing order. I agree it would be better to optionally put i.zero2null into r.import before reprojecting. Will create a corresponding PR

@neteler
Copy link
Member

neteler commented Dec 11, 2021

@griembauer pls rebase this PR to the (new) grass8 branch

@griembauer
Copy link
Contributor Author

This PR can be closed as the fix should be added to r.import (see @mmetz comment)

@griembauer griembauer closed this Dec 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants