TileDB: add support for building overviews on rasters stored as TileDB groups #9837

rouault · 2024-05-02T17:06:24Z

Supports the CREATE_GROUP=YES creation option to create TileDB raster datasets as a TileDB group, to make it cleaner to store overview within it.

See https://github.com/rouault/gdal/blob/tiledb_overview/frmts/tiledb/overview_model.rst for a description on how TileDB overviews are stored.

@normanb / @sgillies : this is mostly based on https://docs.google.com/document/d/1B-NizsoJn79W_RV3ujg6K1D5xJmWEja29Bv3323uYCU/edit#heading=h.p8z7kqfrb9st (with the removal of creating overviews on non-group dataset)

sgillies

@rouault @normanb I'm new to this work, made a quick scan this afternoon. I've got a few suggestions for dataset type names and group/array layout. Norman and I will also sync on this so that we're of one mind on it 😄

sgillies · 2024-05-08T22:21:15Z

frmts/tiledb/overview_model.rst

+as a TileDB array whose name is the base name of the group, suffixed with ``_0``
+
+The first overview level created is a TileDB array, placed within that group,
+and whose name is the base name of the group, suffixed with ``_1``. And so on.


@rouault @normanb using the stem of the group name as a part of the array names makes the entire structure fragile. If a group is renamed or moved, the implicit links between group and arrays are broken.

What would you think about a generic prefix such as "pyr" or no prefix at all?

In other words, this:

$ ls -al byte_group.tiledb drwxr-xr-x 6 even even 4096 mai 2 02:13 ./ drwxrwxr-x 57 even even 86016 mai 2 01:42 ../ drwxr-xr-x 8 even even 4096 mai 1 17:17 pyr_0/ <== Full resolution dataset drwxr-xr-x 8 even even 4096 mai 2 02:13 pyr_1/ <== First overview level drwxr-xr-x 8 even even 4096 mai 2 02:13 pyr_2/ <== Second overview level drwxr-xr-x 2 even even 4096 mai 2 02:13 __group/ drwxr-xr-x 2 even even 4096 mai 2 02:13 __meta/ -rw-r--r-- 1 even even 0 mai 1 17:17 __tiledb_group.tdb

or

$ ls -al byte_group.tiledb drwxr-xr-x 6 even even 4096 mai 2 02:13 ./ drwxrwxr-x 57 even even 86016 mai 2 01:42 ../ drwxr-xr-x 8 even even 4096 mai 1 17:17 0/ <== Full resolution dataset drwxr-xr-x 8 even even 4096 mai 2 02:13 1/ <== First overview level drwxr-xr-x 8 even even 4096 mai 2 02:13 2/ <== Second overview level drwxr-xr-x 2 even even 4096 mai 2 02:13 __group/ drwxr-xr-x 2 even even 4096 mai 2 02:13 __meta/ -rw-r--r-- 1 even even 0 mai 1 17:17 __tiledb_group.tdb

I'm OK with that proposed change

sgillies · 2024-05-08T22:53:52Z

doc/source/drivers/raster/tiledb.rst

+      as a TileDB array. When creating overviews, they will be stored in an
+      auxiliary TileDB group, located at the same level as the full resolution
+      array, with an URI which is the one of the full resolution array with an
+      additional ``.ovr`` suffix.


While it would be handy to be able to add sidecar overviews to an existing array, I'm not sure we want people to create these just because it's possible. What do you think, @normanb ?

sgillies · 2024-05-08T22:56:34Z

frmts/tiledb/overview_model.rst

+
+The group has the following TileDB metadata items:
+
+* ``dataset_type``: set to ``raster``


@rouault @normanb I would like to propose raster_pyr_v1 ("Raster Pyramid Version 1") for the dataset type value. "v1" because I think we'll want to change the group/array layout down the road, and "pyr" to indicate that this is a raster pyramid. raster could remain for single-array rasters with or without a sidecar group.

I'm fine with that change, and the below one too

sgillies · 2024-05-08T23:02:13Z

frmts/tiledb/overview_model.rst

+
+The group has the following TileDB metadata item:
+
+* ``dataset_type``: set to ``raster_overviews``


Maybe dataset_type: raster_ovr_v1 with the same naming change as I proposed for raster_pyr_v1?

rouault · 2024-05-15T22:14:42Z

@sgillies @normanb Did you sync together regarding Sean's above suggestions?

sgillies · 2024-05-22T16:45:02Z

@rouault sorry for the delay! We did discuss and here are the conclusions:

Overview arrays should be named l_0, l_1, etc.
Dataset_type metadata should remain raster.
We do not desire a sidecar .ovr group and the TileDB platform will not support it, so it would be best to delete that format variant.
We'd like to see CREATE_GROUP=YES be the default. Groups will be favored by the TileDB platform. We understand that this means a group with a single array for users who don't request overviews.

…B_ATTRIBUTE creation option, when the block size is not a multiple of the raster size

…y mode

…r+b ; GTiff: turn VSIError into CPLError on creation

…LDebugOnly()/CPLAssert()

…removed per 130560b Even if no longer use deprecated features, the mere fact of including TileDB headers that declare deprecated methods cause warnings to be emitted without the suppression.

… in that mode

…up mode

… still functional

…t in group mode" This reverts commit a6a0c7f.

rouault · 2024-05-23T15:03:52Z

We did discuss and here are the conclusions:

I've updated the pull request with those changes, starting at commit "Revert "TileDB raster: implement BuildOverviews() for base dataset not in group mode").
As the CREATE_GROUP creation option is common to the raster and vector sides of the driver, I found it to be perhaps simpler to change its default to YES for both (at least to be able to present a default value in the XML metadata of GDAL_DMD_CREATIONOPTIONLIST), so I also changed it for vectors. I could revert that change if needed.

coveralls · 2024-05-23T15:48:50Z

coverage: 69.133%. remained the same
when pulling 8c64193 on rouault:tiledb_overview
into 12dab86 on OSGeo:master.

rouault added this to the 3.10.0 milestone May 2, 2024

sgillies reviewed May 8, 2024

View reviewed changes

rouault added 18 commits May 23, 2024 14:43

TileDBRasterBand::IRasterIO(): avoid reading out-of-raster with TILED…

e125fe5

…B_ATTRIBUTE creation option, when the block size is not a multiple of the raster size

TileDB: CreateCopy(): do not force returned dataset to be in read-onl…

c0b970f

…y mode

Add a more C++ friendly GDALRegenerateOverviewsMultiBand() signature

392181b

Add VSIToCPLErrorWithMsg()

0fa201c

/vsis3/: avoid emitting a CPLError() when bSetError=false and access=…

fec6d66

…r+b ; GTiff: turn VSIError into CPLError on creation

cpl_error.h: provide 'do {} while(0)' expansion for empty CPLDebug/CP…

a69d46f

…LDebugOnly()/CPLAssert()

TileDB: clarify code comment

f1dd99c

include_tiledb.h: reintroduce MSVC 4996 warning suppression that was …

b5a5079

…removed per 130560b Even if no longer use deprecated features, the mere fact of including TileDB headers that declare deprecated methods cause warnings to be emitted without the suppression.

TileDB: catch potential TileDB exceptions in Open() and IRasterIO()

a9bd361

TileDB raster: honor CREATE_GROUP=YES, and implement BuildOverviews()…

273d5d0

… in that mode

TileDB raster: implement BuildOverviews() for base dataset not in gro…

a6a0c7f

…up mode

TileDB: document existing open and creation options

35ab7ad

TileDB: document new CREATE_GROUP creation option

4e36c6e

TileDB: add a overview_model.rst design document

894c200

TileDB tests: add a test to check deprecated GeoTIFF side-car .ovr is…

849bfde

… still functional

Revert "TileDB raster: implement BuildOverviews() for base dataset no…

c6c2b79

…t in group mode" This reverts commit a6a0c7f.

TileDB raster: change name of overview level arrays to 'l_{level}'

1252074

TileDB: overview_model.rst: update it with latest changes

9014131

rouault force-pushed the tiledb_overview branch from bed01cc to c51ff82 Compare May 23, 2024 14:59

rouault changed the title ~~TileDB: add support for building overviews stored as TileDB arrays~~ TileDB: add support for building overviews stored as TileDB groups May 23, 2024

rouault changed the title ~~TileDB: add support for building overviews stored as TileDB groups~~ TileDB: add support for building overviews on rasters stored as TileDB groups May 23, 2024

rouault added 3 commits May 23, 2024 17:55

TileDB: change CREATE_GROUP default value to YES for rasters

2b304a0

TileDB: advertize GDAL_DCAP_CREATE_LAYER

0bda2b4

TileDB: change CREATE_GROUP default value to YES for vectors

bdd9694

TileDB: change CREATE_GROUP default value to YES in metadata

8c64193

rouault force-pushed the tiledb_overview branch from c51ff82 to 8c64193 Compare May 23, 2024 15:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TileDB: add support for building overviews on rasters stored as TileDB groups #9837

TileDB: add support for building overviews on rasters stored as TileDB groups #9837

rouault commented May 2, 2024 •

edited

sgillies left a comment

sgillies May 8, 2024

rouault May 8, 2024

sgillies May 8, 2024

sgillies May 8, 2024

rouault May 8, 2024

sgillies May 8, 2024

rouault commented May 15, 2024

sgillies commented May 22, 2024

rouault commented May 23, 2024

coveralls commented May 23, 2024 •

edited


		The group has the following TileDB metadata items:

		* ``dataset_type``: set to ``raster``


		The group has the following TileDB metadata item:

		* ``dataset_type``: set to ``raster_overviews``

TileDB: add support for building overviews on rasters stored as TileDB groups #9837

Are you sure you want to change the base?

TileDB: add support for building overviews on rasters stored as TileDB groups #9837

Conversation

rouault commented May 2, 2024 • edited

sgillies left a comment

Choose a reason for hiding this comment

sgillies May 8, 2024

Choose a reason for hiding this comment

rouault May 8, 2024

Choose a reason for hiding this comment

sgillies May 8, 2024

Choose a reason for hiding this comment

sgillies May 8, 2024

Choose a reason for hiding this comment

rouault May 8, 2024

Choose a reason for hiding this comment

sgillies May 8, 2024

Choose a reason for hiding this comment

rouault commented May 15, 2024

sgillies commented May 22, 2024

rouault commented May 23, 2024

coveralls commented May 23, 2024 • edited

rouault commented May 2, 2024 •

edited

coveralls commented May 23, 2024 •

edited