#3084: i.segment: notnullcells defined as long too limited
-------------------------------------+-------------------------
Reporter: mlennert | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.5
Component: Imagery | Version: unspecified
Keywords: i.segment variable type | CPU: Unspecified
Platform: Unspecified |
-------------------------------------+-------------------------
In iseg.h, notnullcells is defined as long. On Windows this has a range of
–2147483648 through 2147483647.
We are working on a region that has over 7 billion pixels and so the
nonnullcells variable overflows, becomes negative and i.segment fails with
"insufficient number of non-null cells".
Two reflections:
* Shouldn't this be unsigned ?
* Maybe a long long would be safer, seeing that pixel numbers don't stop
increasing.
#3084: i.segment: notnullcells defined as long too limited
--------------------------+-------------------------------------
Reporter: mlennert | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.5
Component: Imagery | Version: unspecified
Resolution: | Keywords: i.segment variable type
CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------------------
Comment (by mmetz):
Replying to [ticket:3084 mlennert]:
> In iseg.h, notnullcells is defined as long. On Windows this has a range
of –2147483648 through 2147483647.
>
> We are working on a region that has over 7 billion pixels and so the
nonnullcells variable overflows, becomes negative and i.segment fails with
"insufficient number of non-null cells".
>
> Two reflections:
> * Shouldn't this be unsigned ?
> * Maybe a long long would be safer, seeing that pixel numbers don't stop
increasing.
For nonnullcells, long long would be ok. But i.segment is not prepared to
handle raster maps with more than 2147483647 cells, at least the region
IDs would also need to be changed, or handled differently. Variables for
region ID as well as the region ID output is 32 bit signed integer, so
there will be another integer overflow, particularly because in the
beginning each cell gets a unique region ID.
Replying to [comment:1 mmetz]:
> Replying to [ticket:3084 mlennert]:
> > In iseg.h, notnullcells is defined as long. On Windows this has a
range of –2147483648 through 2147483647.
> >
> > We are working on a region that has over 7 billion pixels and so the
nonnullcells variable overflows, becomes negative and i.segment fails with
"insufficient number of non-null cells".
> >
> > Two reflections:
> > * Shouldn't this be unsigned ?
> > * Maybe a long long would be safer, seeing that pixel numbers don't
stop increasing.
>
> For nonnullcells, long long would be ok. But i.segment is not prepared
to handle raster maps with more than 2147483647 cells, at least the region
IDs would also need to be changed, or handled differently. Variables for
region ID as well as the region ID output is 32 bit signed integer, so
there will be another integer overflow, particularly because in the
beginning each cell gets a unique region ID.
Ok, so I'll retype the ticket to enhancement request and change the title
to reflect the demand for i.segment to handle over 2147483647 cells. Other
than changing variable types, does this entail any other changes ?
#3084: i.segment: allow more than 2147483647 cells
--------------------------+-------------------------------------
Reporter: mlennert | Owner: grass-dev@…
Type: enhancement | Status: new
Priority: normal | Milestone: 7.2.0
Component: Imagery | Version: unspecified
Resolution: | Keywords: i.segment variable type
CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------------------
Comment (by mmetz):
Replying to [comment:2 mlennert]:
> Replying to [comment:1 mmetz]:
> > Replying to [ticket:3084 mlennert]:
> > > In iseg.h, notnullcells is defined as long. On Windows this has a
range of –2147483648 through 2147483647.
> > >
> > > We are working on a region that has over 7 billion pixels and so the
nonnullcells variable overflows, becomes negative and i.segment fails with
"insufficient number of non-null cells".
> > >
> > > Two reflections:
> > > * Shouldn't this be unsigned ?
> > > * Maybe a long long would be safer, seeing that pixel numbers don't
stop increasing.
> >
> > For nonnullcells, long long would be ok. But i.segment is not prepared
to handle raster maps with more than 2147483647 cells, at least the region
IDs would also need to be changed, or handled differently. Variables for
region ID as well as the region ID output is 32 bit signed integer, so
there will be another integer overflow, particularly because in the
beginning each cell gets a unique region ID.
>
> Ok, so I'll retype the ticket to enhancement request and change the
title to reflect the demand for i.segment to handle over 2147483647 cells.
Other than changing variable types, does this entail any other changes ?
Most importantly, a GRASS raster format for 64-bit signed integer. Note
that GDAL does not support 64-bit signed or unsigned integers. The reason
is probably that a portable implementation of 64-bit integers is not so
easy. Regarding GRASS raster processing, the need for 64-bit integers
usually arises only for raster maps with more than 2,147,483,647 cells
which in turn usually requires Large File Support (LFS). Therefore the
check for the availability of a 64-bit integer could be coupled to LFS. If
support for 64-bit signed integer raster maps (say, LCELL) could be added
to GRASS, users would need to stick to GRASS since GDAL raster export of
64-bit integers is not available. Interesting idea. In r68944, I have
implemented for i.segment conditional support for 64-bit signed integers:
long long int or a 64-bit off_t or a 64-bit long int must be available.
r68944 does not solve but alleviates the problem if the current region
comprises more than 2,147,483,647 cells. r68944 also adds checks for
integer overflow. The handling of initial region IDs has been changed and
the module is now about 6x faster for regions > 10 million cells,
depending on the threshold value for region growing.
#3084: i.segment: allow more than 2147483647 cells
--------------------------+-------------------------------------
Reporter: mlennert | Owner: grass-dev@…
Type: enhancement | Status: new
Priority: normal | Milestone: 7.2.0
Component: Imagery | Version: unspecified
Resolution: | Keywords: i.segment variable type
CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------------------
Comment (by cmbarton):
Just a comment about down-the-line issues. Using 64bit versions of
dependencies like GDAL underscores the importance of getting the GUI to
64bit so we don't have to deal with the messiness of compiling dual
architecture. This means moving the GUI to wxPython 3.x or wxPython
Phoenix. IIRC, this is an issue for Windows as well as Mac currently.
There is a test version of GRASS 7.3 64bit with wxPython 3 available now.
It seems like there are not many issues to fix, but testing is needed to
ID all of them. I don't know where we are with Windows on this. Anna is
looking into moving to the Phoenix build but as of May, some of the GUI
controls we need are still missing from this build.
#3084: i.segment: allow more than 2147483647 cells
--------------------------+-------------------------------------
Reporter: mlennert | Owner: grass-dev@…
Type: enhancement | Status: new
Priority: normal | Milestone: 7.4.1
Component: Imagery | Version: unspecified
Resolution: | Keywords: i.segment variable type
CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------------------
Comment (by mmetz):
Replying to [comment:10 martinl]:
> What is status of this issue?
As of r68944, i.segment supports regions with more than 2,147,483,647
cells, but the number of final objects must not exceed 2,147,483,647. This
can only be solved by introducing a new 64 bit integer data type in
addition to CELL, FCELL, DCELL, something for GRASS 8 maybe.