Opened 20 months ago

Last modified 3 months ago

#3815 new defect

compression: libzstd should be a default config and a requirement if ZSTD is default compression method

Reported by: mlennert Owned by: grass-dev@…
Priority: normal Milestone: 7.6.2
Component: Raster Version: svn-trunk
Keywords: compression zstd zlib Cc:
CPU: Unspecified Platform: Unspecified

Description

I just came upon the problem that in my grass installation ZSTD is configured and I thus worked with raster data which was automatically compressed using ZSTD, but when I transferred the data to our HPC system, I realized that grass there is not configure with ZSTD and the latter is currently not available on the system.

As the r.compress man page says, ZSTD is the default compressor if available. However, when I check in current trunk's configure, it says:

  --with-zstd             support Zstandard functionality (default: no)"

Does that mean that even if libzstd is available on a system, GRASS is not compiled with support for it, unless --with-zstd is set ?

I find this situation a bit uncomfortable as it can lead to situations where the user creates data without actively thinking about the compression method and this data is the unusable elsewhere.

I would plead, thus, for libzstd to become a compulsory requirement for GRASS GIS, and for the configure script to default to --with-zstd=yes.

Or the default compression method should be put back to ZLIB.

Change History (19)

comment:1 in reply to:  description ; Changed 20 months ago by mmetz

Replying to mlennert:

I just came upon the problem that in my grass installation ZSTD is configured and I thus worked with raster data which was automatically compressed using ZSTD, but when I transferred the data to our HPC system, I realized that grass there is not configure with ZSTD and the latter is currently not available on the system.

As the r.compress man page says, ZSTD is the default compressor if available. However, when I check in current trunk's configure, it says:

  --with-zstd             support Zstandard functionality (default: no)"

Does that mean that even if libzstd is available on a system, GRASS is not compiled with support for it, unless --with-zstd is set ?

yes

I find this situation a bit uncomfortable as it can lead to situations where the user creates data without actively thinking about the compression method and this data is the unusable elsewhere.

I would plead, thus, for libzstd to become a compulsory requirement for GRASS GIS, and for the configure script to default to --with-zstd=yes.

Done in trunk r74391. The default is now --with-zstd=yes, you can disable ZSTD support with --with-zstd=no. To be backported I guess.

comment:2 in reply to:  1 ; Changed 20 months ago by mlennert

Replying to mmetz:

Replying to mlennert:

I just came upon the problem that in my grass installation ZSTD is configured and I thus worked with raster data which was automatically compressed using ZSTD, but when I transferred the data to our HPC system, I realized that grass there is not configure with ZSTD and the latter is currently not available on the system.

As the r.compress man page says, ZSTD is the default compressor if available. However, when I check in current trunk's configure, it says:

  --with-zstd             support Zstandard functionality (default: no)"

Does that mean that even if libzstd is available on a system, GRASS is not compiled with support for it, unless --with-zstd is set ?

yes

I find this situation a bit uncomfortable as it can lead to situations where the user creates data without actively thinking about the compression method and this data is the unusable elsewhere.

I would plead, thus, for libzstd to become a compulsory requirement for GRASS GIS, and for the configure script to default to --with-zstd=yes.

Done in trunk r74391. The default is now --with-zstd=yes, you can disable ZSTD support with --with-zstd=no. To be backported I guess.

+1 for backporting.

Should libzstd be added to REQUIREMENTS.html ?

comment:3 in reply to:  2 ; Changed 20 months ago by mmetz

Replying to mlennert:

Replying to mmetz:

Replying to mlennert:

[...]

I would plead [...] for libzstd to become a compulsory requirement for GRASS GIS, and for the configure script to default to --with-zstd=yes.

Done in trunk r74391. The default is now --with-zstd=yes, you can disable ZSTD support with --with-zstd=no. To be backported I guess.

+1 for backporting.

Done in relbr76 r74398

Should libzstd be added to REQUIREMENTS.html ?

Yes, done in r74396,9 (trunk, relbr76)

comment:4 Changed 20 months ago by martinl

Milestone: 7.8.07.6.2

comment:5 in reply to:  3 ; Changed 20 months ago by neteler

Replying to mmetz:

Replying to mlennert:

+1 for backporting.

Done in relbr76 r74398

Just FYI: This broke the manual-building cronjobs on the grass.osgeo.org server but I fixed it by now actively disabling zstd in the configure step (Debian Jessie doesn't have zstd yet and #3486 = compile in a docker container is still open).

In any case, the manuals on the server are now back.

comment:6 in reply to:  5 ; Changed 20 months ago by mmetz

Replying to neteler:

Replying to mmetz:

Replying to mlennert:

+1 for backporting.

Done in relbr76 r74398

Just FYI: This broke the manual-building cronjobs on the grass.osgeo.org server but I fixed it by now actively disabling zstd in the configure step (Debian Jessie doesn't have zstd yet and #3486 = compile in a docker container is still open).

A good reason to not make zstd a compulsory requirement. Debian Jessie has long-term support until 30 June 2020, and GRASS 7.x should IMHO run on this system (with appropriate configure options).

comment:7 in reply to:  6 ; Changed 19 months ago by mmetz

Replying to mmetz:

Replying to neteler:

Replying to mmetz:

Replying to mlennert:

+1 for backporting.

Done in relbr76 r74398

Just FYI: This broke the manual-building cronjobs on the grass.osgeo.org server but I fixed it by now actively disabling zstd in the configure step (Debian Jessie doesn't have zstd yet and #3486 = compile in a docker container is still open).

A good reason to not make zstd a compulsory requirement. Debian Jessie has long-term support until 30 June 2020, and GRASS 7.x should IMHO run on this system (with appropriate configure options).

I think there was some misunderstanding from my side. For maximum compatibility of GRASS raster data, the default compression should indeed be ZLIB, even if ZSTD is available. I made ZSTD the default if available in order to get it tested.

There was one issue reported where raster data compressed with ZSTD 1.3.6 could not be read with ZSTD 1.3.1, which remains a mystery.

What are the opinions on changing the default compression back to ZLIB even if ZSTD is available?

+1 from Moritz Lennert as I understand

0 from me

comment:8 in reply to:  7 Changed 19 months ago by martinl

Replying to mmetz:

+1 from Moritz Lennert as I understand

0 from me

+1

comment:9 in reply to:  7 Changed 19 months ago by mlennert

Replying to mmetz:

Replying to mmetz:

Replying to neteler:

Replying to mmetz:

Replying to mlennert:

+1 for backporting.

Done in relbr76 r74398

Just FYI: This broke the manual-building cronjobs on the grass.osgeo.org server but I fixed it by now actively disabling zstd in the configure step (Debian Jessie doesn't have zstd yet and #3486 = compile in a docker container is still open).

A good reason to not make zstd a compulsory requirement. Debian Jessie has long-term support until 30 June 2020, and GRASS 7.x should IMHO run on this system (with appropriate configure options).

I think there was some misunderstanding from my side. For maximum compatibility of GRASS raster data, the default compression should indeed be ZLIB, even if ZSTD is available. I made ZSTD the default if available in order to get it tested.

There was one issue reported where raster data compressed with ZSTD 1.3.6 could not be read with ZSTD 1.3.1, which remains a mystery.

What are the opinions on changing the default compression back to ZLIB even if ZSTD is available?

+1 from Moritz Lennert as I understand

Yes.

comment:10 Changed 19 months ago by mmetz

In 74426:

libgis: make ZLIB the default compressor even if ZSTD is available, see #3815

comment:11 in reply to:  10 Changed 19 months ago by mmetz

Replying to mmetz:

In 74426:

libgis: make ZLIB the default compressor even if ZSTD is available, see #3815

This change means that newly created raster maps are by default compressed with ZLIB, unless the environment variable GRASS_COMPRESSOR is set. Previously created raster maps compressed with ZSTD are readable as long as GRASS 7.6 is compiled with ZSTD.

Trunk still uses ZSTD as default if available.

comment:12 Changed 19 months ago by neteler

Please note that this change in 7.6 contradicts our release announcement:

https://trac.osgeo.org/grass/wiki/Release/7.6.0-News#Librarychanges

"libraster: ZSTD is now the default compression if available"

comment:13 in reply to:  12 ; Changed 19 months ago by mmetz

Replying to neteler:

Please note that this change in 7.6 contradicts our release announcement:

https://trac.osgeo.org/grass/wiki/Release/7.6.0-News#Librarychanges

"libraster: ZSTD is now the default compression if available"

The release announcement for 7.6.2 should then include something like

The default compression has been changed back to ZLIB, even if ZSTD compression is available, because not all currently supported systems provide ZSTD with standard package management tools, and because problems were encountered regarding compatibility with different ZSTD 1.3.x versions.

comment:14 in reply to:  13 Changed 19 months ago by neteler

Replying to mmetz:

Replying to neteler:

Please note that this change in 7.6 contradicts our release announcement:

https://trac.osgeo.org/grass/wiki/Release/7.6.0-News#Librarychanges

"libraster: ZSTD is now the default compression if available"

The release announcement for 7.6.2 should then include something like

The default compression has been changed back to ZLIB, even if ZSTD compression is available, because not all currently supported systems provide ZSTD with standard package management tools, and because problems were encountered regarding compatibility with different ZSTD 1.3.x versions.

Added in new draft: https://trac.osgeo.org/grass/wiki/Release/7.6.2-News

comment:15 Changed 13 months ago by neteler

As a reference, the related relbranch76 change in GH: https://github.com/OSGeo/grass/commit/d3e60da04e0e5f7be151ee3a51deb8c6135e0527

comment:16 Changed 5 months ago by neteler

For the record: in G7.8 and master ZSTD is the default compression:

As we didn't get any complaints with that, I suggest to keep it as is.

comment:17 Changed 5 months ago by wenzeslaus

Problems reported between Oct 31, 2019 and now:

Generally speaking, changing the default in minor version became challenge for users sharing data (locations, packs) with other users (...choice of raster compression method...) since the requirement was version 7.6 compiled with newly introduced ZSTD. Expected issues between major versions, but not minor ones.

Even more problems were created by compatibility issues between different ZSTD versions.

comment:19 in reply to:  18 Changed 3 months ago by neteler

Replying to annakrat:

Another reported problem: https://gis.stackexchange.com/questions/368823/r-grow-distance-with-r-mask-returns-errors-on-a-global-scale-but-not-with-a-sma

Yes, but reported with GRASS GIS 7.6 and ZSTD 1.3.2.0.

Currently ZSTD 1.4.4-1 is shipped in OSGeo4W).

Note: See TracTickets for help on using tickets.