[GRASS-dev] [GRASS GIS] #3815: compression: libzstd should be a default config and a requirement if ZSTD is default compression method

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
-----------------------------------+-------------------------
Reporter: mlennert | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 7.8.0
Component: Raster | Version: svn-trunk
Keywords: compression zstd zlib | CPU: Unspecified
Platform: Unspecified |
-----------------------------------+-------------------------
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.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.8.0
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by mmetz):

Replying to [ticket:3815 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.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:1&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.8.0
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by mlennert):

Replying to [comment:1 mmetz]:
> Replying to [ticket:3815 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 ?

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:2&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.8.0
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by mmetz):

Replying to [comment:2 mlennert]:
> Replying to [comment:1 mmetz]:
> > Replying to [ticket:3815 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)

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:3&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.2
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------
Changes (by martinl):

* milestone: 7.8.0 => 7.6.2

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:4&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.2
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by neteler):

Replying to [comment:3 mmetz]:
> Replying to [comment:2 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.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:5&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.2
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by mmetz):

Replying to [comment:5 neteler]:
> Replying to [comment:3 mmetz]:
> > Replying to [comment:2 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).

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:6&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.2
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by mmetz):

Replying to [comment:6 mmetz]:
> Replying to [comment:5 neteler]:
> > Replying to [comment:3 mmetz]:
> > > Replying to [comment:2 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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:7&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.2
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by martinl):

Replying to [comment:7 mmetz]:

> +1 from Moritz Lennert as I understand
>
> 0 from me

+1

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:8&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.2
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by mlennert):

Replying to [comment:7 mmetz]:
> Replying to [comment:6 mmetz]:
> > Replying to [comment:5 neteler]:
> > > Replying to [comment:3 mmetz]:
> > > > Replying to [comment:2 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.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:9&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.2
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by mmetz):

In [changeset:"74426" 74426]:
{{{
#!CommitTicketReference repository="" revision="74426"
libgis: make ZLIB the default compressor even if ZSTD is available, see
#3815
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:10&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.2
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by mmetz):

Replying to [comment:10 mmetz]:
> In [changeset:"74426" 74426]:
> {{{
> #!CommitTicketReference repository="" revision="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.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:11&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.2
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (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"

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:12&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.2
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by mmetz):

Replying to [comment:12 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.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:13&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.2
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by neteler):

Replying to [comment:13 mmetz]:
> Replying to [comment:12 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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:14&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.2
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by neteler):

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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:15&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.2
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by neteler):

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

*
https://github.com/OSGeo/grass/blob/releasebranch_7_8/lib/gis/compress.c#L126
* https://github.com/OSGeo/grass/blob/master/lib/gis/compress.c#L126

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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:16&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.2
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by wenzeslaus):

Problems reported between Oct 31, 2019 and now:

* [https://lists.osgeo.org/pipermail/grass-dev/2019-December/093800.html
ZSTD error]
* [https://lists.osgeo.org/pipermail/grass-dev/2020-May/094351.html ZSTD
compression error on Windows]

Generally speaking, changing the default in minor version became challenge
for users sharing data (locations, packs) with other users
([https://lists.osgeo.org/pipermail/grass-dev/2019-December/093802.html
...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.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:17&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.2
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by 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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:18&gt;
GRASS GIS <https://grass.osgeo.org>

#3815: compression: libzstd should be a default config and a requirement if ZSTD
is default compression method
--------------------------+-----------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.2
Component: Raster | Version: svn-trunk
Resolution: | Keywords: compression zstd zlib
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------

Comment (by neteler):

Replying to [comment:18 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).

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3815#comment:19&gt;
GRASS GIS <https://grass.osgeo.org>