open_stds: check for unknown band references

[landam]

t.rast.mapcalc on unknown band reference (in this case S2_B4):

t.rast.mapcalc inputs=s2_tile_5606.S2_8A,s2_tile_5606.S2_B4 output=ndvi basename=ndvi expression="float(s2_tile_5606.S2_8A - s2_tile_5606.S2_4) / (s2_tile_5606.S2_8A + s2_tile_5606.S2_4)" --o 

fails with unclear error message:

Starting temporal sampling...
Traceback (most recent call last):
Starting mapcalc   File "/home/martin/src/grass/dist.x86_64-pc-linux-gnu/scripts/t.rast.mapcalc", line 119, in <module>
computation...
    main()
  File "/home/martin/src/grass/dist.x86_64-pc-linux-gnu/scripts/t.rast.mapcalc", line 102, in main
    tgis.dataset_mapcalculator(
  File "/home/martin/src/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/temporal/mapcalc.py", line 248, in dataset_mapcalculator
    if map_matrix[j][i] is None:
IndexError: list index out of range

This PR introduces a proper error message:

ERROR: Unknown <S2_B4> band reference. Run g.bands module to list supported
bands.

[metzm]

In the example above, the expression refers to s2_tile_5606.S2_4, not s2_tile_5606.S2_B4. Thus the error message

ERROR: Unknown <S2_B4> band reference. Run g.bands module to list supported
bands.

is probably not correct.

Also note that for t.rast.mapcalc the band names in TGIS matter, e.g. created with t.register. These band names can also be custom names like ndvi, mask, SLC, etc.

Somewhat related and needed for openEO: it would be great if t.info would list all band names in a STRDS. That would make it easier for users to verify the list of available band names.

[veroandreo]

In the example above, the expression refers to s2_tile_5606.S2_4, not s2_tile_5606.S2_B4. Thus the error message

ERROR: Unknown <S2_B4> band reference. Run g.bands module to list supported
bands.

is probably not correct.

I'm confused. The "right" band names as defined in g.bands are S2_4 and S2_8A, hence in the t.rast.mapcalc call above, the use of S2_B4 is wrong, that band name is unknown/not supported. So, in those terms, I find the error message is fine. What would be your suggestion @metzm ?

Also note that for t.rast.mapcalc the band names in TGIS matter, e.g. created with t.register. These band names can also be custom names like ndvi, mask, SLC, etc.

I fully support this, there might be other arbitrary band names that should be considered. Indeed, how does this relate to @marisn contributions?

Somewhat related and needed for openEO: it would be great if t.info would list all band names in a STRDS. That would make it easier for users to verify the list of available band names.

Agreed! In general, I'd pledge for a better integration of the band reference concept into the temporal framework.

[metzm]

I'm confused. The "right" band names as defined in g.bands are S2_4 and S2_8A, hence in the t.rast.mapcalc call above, the use of S2_B4 is wrong, that band name is unknown/not supported. So, in those terms, I find the error message is fine. What would be your suggestion @metzm ?

You are right, the use of S2_B4 as band suffix to the list of input strds's is wrong and is not used in the actual mapcalc expression

expression="float(s2_tile_5606.S2_8A - s2_tile_5606.S2_4) / (s2_tile_5606.S2_8A + s2_tile_5606.S2_4)" 

I have not tested it, but preferably the band names would be appended as suffixes only to the expression, not to the list of input strds's, e.g.

t.rast.mapcalc inputs=s2_tile_5606 \
               output=ndvi basename=ndvi \
               expression="float(s2_tile_5606.S2_8A - s2_tile_5606.S2_4) / (s2_tile_5606.S2_8A + s2_tile_5606.S2_4)"

should work. If not, that could be a relatively easy enhancement to t.rast.mapcalc.

[landam]

In the example above, the expression refers to s2_tile_5606.S2_4, not s2_tile_5606.S2_B4. Thus the error message

ERROR: Unknown <S2_B4> band reference. Run g.bands module to list supported
bands.

is probably not correct.

@metzm The message refers to inputs=s2_tile_5606.S2_8A,s2_tile_5606.S2_B4 not to the expression. Does it make sense than to you?

[landam]

I have not tested it, but preferably the band names would be appended as suffixes only to the expression, not to the list of input strds's, e.g.
should work. If not, that could be a relatively easy enhancement to t.rast.mapcalc.

This would for a new PR.

[marisn]

In the example above, the expression refers to s2_tile_5606.S2_4, not s2_tile_5606.S2_B4. Thus the error message

ERROR: Unknown <S2_B4> band reference. Run g.bands module to list supported
bands.

is probably not correct.

From the C library point of view, this message is incorrect as S2_B4 is as valid band reference as NDVI or slope_deg. Still common C library rules do not apply to t.* modules (intentionally?).
https://github.com/OSGeo/grass/blob/main/lib/raster/raster_metadata.c#L136

[landam]

From the C library point of view, this message is incorrect as S2_B4 is as valid band reference as NDVI or slope_deg. Still common C library rules do not apply to t.* modules (intentionally?).
https://github.com/OSGeo/grass/blob/main/lib/raster/raster_metadata.c#L136

@metzm Supported band references are provided by g.bands module (or by relevant Python API - https://github.com/OSGeo/grass/blob/main/python/grass/bandref/reader.py#L182) not by C library.

[landam]

Rast_legal_bandref should be used when registering new band ref to the registry to my knowledge.

[marisn]

@metzm Supported band references are provided by g.bands module (or by relevant Python API - https://github.com/OSGeo/grass/blob/main/python/grass/bandref/reader.py#L182) not by C library.

This then applies to t.* modules only, as rasters can have any band reference. That includes known and unknown to g.bands. If it is not known to g.bands, it simply lacks any extra metadata but still can serve as an identifier, as it is used by i.* modules.

[landam]

This then applies to t.* modules only, as rasters can have any band reference. That includes known and unknown to g.bands. If it is not known to g.bands, it simply lacks any extra metadata but still can serve as an identifier, as it is used by i.* modules.

OK, unfortunately I am not able to track all changes related to band reference support. The original idea was that g.bands will be used as registry of supported band reference. That has been changed apparently.

BTW, @marisn recently added support for band reference to r.support duplicates i.bands module. So feel free to remove i.bands module.

[landam]

@metzm I changed PR based on your notes. Now t.rast.mapcalc ends up with slightly different message:

t.rast.mapcalc inputs=s2_tile_5606.S2_8A,s2_tile_5606.S2_B4 output=ndvi basename=ndvi expression="float(s2_tile_5606.S2_8A - s2_tile_5606.S2_4) / (s2_tile_5606.S2_8A + s2_tile_5606.S2_4)" --o
Starting temporal sampling...
No maps registered in input dataset <s2_tile_5606.S2_B4>

[marisn]

OK, unfortunately I am not able to track all changes related to band reference support. The original idea was that g.bands will be used as registry of supported band reference. That has been changed apparently.

It was too restrictive. Markus M. even considered band reference naming scheme to be too restrictive – thus now anything goes. Individual modules or library functions still can enforce presence of band reference entry in metadata managed by g.bands iff there is functionality depending on a particular band (e.g. a specific band for cloud detection). Current implementation of band references allows to assign a semantic tag to any data not only satellite data. E.g. there is no satellite with a NDVI band but one might want to include NDVI in classification of imagery.

[landam]

It was too restrictive. Markus M. even considered band reference naming scheme to be too restrictive – thus now anything goes. Individual modules or library functions still can enforce presence of band reference entry in metadata managed by g.bands iff there is functionality depending on a particular band (e.g. a specific band for cloud detection). Current implementation of band references allows to assign a semantic tag to any data not only satellite data. E.g. there is no satellite with a NDVI band but one might want to include NDVI in classification of imagery.

OK, make sense to me.

[metzm]

My motivation for custom band names comes from the need in openEO to allow custom band names, even to rename band names. From #1796 :

Rast_legal_bandref() was too restrictive requiring band names of the form <shortcut>_<bandname>. Such band names are not STAC or openEO compatible, see https://github.com/stac-extensions/eo#band-object and https://github.com/stac-extensions/eo#common-band-names. Most importantly, custom band names must be allowed.