[GRASS-dev] i.topo.corr ported to GRASS 7

Hi,

I have more or less ported i.topo.corr to GRASS 7:
grass-addons/grass7/imagery/i.topo.corr/

there are still some initalization issues when opening raster maps
due to the new style in G7. No idea how to fix that, help
would be appreciated.

thanks
Markus

Markus Neteler wrote:

I have more or less ported i.topo.corr to GRASS 7:
grass-addons/grass7/imagery/i.topo.corr/

there are still some initalization issues when opening raster maps
due to the new style in G7. No idea how to fix that, help
would be appreciated.

Can you explain what the issues are?

--
Glynn Clements <glynn@gclements.plus.com>

On Fri, Aug 5, 2011 at 4:22 AM, Glynn Clements <glynn@gclements.plus.com> wrote:

Markus Neteler wrote:

I have more or less ported i.topo.corr to GRASS 7:
grass-addons/grass7/imagery/i.topo.corr/

there are still some initalization issues when opening raster maps
due to the new style in G7. No idea how to fix that, help
would be appreciated.

Can you explain what the issues are?

IMHO the raster map management is too much "old style",
i.e. GRASS 4/5 oriented.

The current version in SVN Addons
http://trac.osgeo.org/grass/browser/grass-addons/grass7/imagery/i.topo.corr
does this:

GRASS 7.0.svn (nc_spm_08):~ > i.topo.corr -i input=myplane_pyr
output=myplane_pyr_illumination basemap=myplane_pyr zenith=45.652668
azimuth=100.552200 method=c-factor
WARNING: Illegal filename <]>. Character <> not allowed.
WARNING: Illegal filename <]>. Character <> not allowed.
ERROR: Unable to open header file for raster map <]@>

Probably more code simplification is needed to leave relevant
things to the GRASS libs rather doing them in the module
itself.

Markus

http://trac.osgeo.org/grass/browser/grass-addons/grass7/imagery/i.topo.corr
does this:

GRASS 7.0.svn (nc_spm_08):~ > i.topo.corr -i input=myplane_pyr
output=myplane_pyr_illumination basemap=myplane_pyr zenith=45.652668
azimuth=100.552200 method=c-factor
WARNING: Illegal filename <]>. Character <> not allowed.
WARNING: Illegal filename <]>. Character <> not allowed.
ERROR: Unable to open header file for raster map <]@>

This is because the Gfile structures didn't have the "name" field
initialised. Try r47464.

--
Glynn Clements <glynn@gclements.plus.com>

On Fri, Aug 5, 2011 at 9:51 PM, Glynn Clements <glynn@gclements.plus.com> wrote:

http://trac.osgeo.org/grass/browser/grass-addons/grass7/imagery/i.topo.corr
does this:

GRASS 7.0.svn (nc_spm_08):~ > i.topo.corr -i input=myplane_pyr
output=myplane_pyr_illumination basemap=myplane_pyr zenith=45.652668
azimuth=100.552200 method=c-factor
WARNING: Illegal filename <]>. Character <> not allowed.
WARNING: Illegal filename <]>. Character <> not allowed.
ERROR: Unable to open header file for raster map <]@>

This is because the Gfile structures didn't have the "name" field
initialised. Try r47464.

Thanks also for the other fixes. I have yet simplified the parser
part again.

Now I get this:

GRASS 7.0.svn (nc_spm_08):~ > g.region rast=myplane_pyr -p
projection: 99 (Lambert Conformal Conic)
zone: 0
datum: nad83
ellipsoid: a=6378137 es=0.006694380022900787
north: 308500
south: 215000
west: 630000
east: 705000
nsres: 250
ewres: 250
rows: 374
cols: 300
cells: 112200

GRASS 7.0.svn (nc_spm_08):~ > i.topo.corr -i input=myplane_pyr
output=myplane_pyr_illumination basemap=myplane_pyr zenith=45.652668
azimuth=100.552200 method=c-factor
ERROR: Input window changed while maps are open for read

Markus

Markus Neteler wrote:

ERROR: Input window changed while maps are open for read

The window shouldn't be changed while any maps are open.

AFAICT, the intention is to perform all processing at the map's native
resolution, right? In which case ...

In main.c, Rast_set_window() should be called before the map is
opened. In illumination.c, G_get_set_window() should be changed to
Rast_get_window().

--
Glynn Clements <glynn@gclements.plus.com>

On Sun, Aug 7, 2011 at 7:07 AM, Glynn Clements <glynn@gclements.plus.com> wrote:

Markus Neteler wrote:

ERROR: Input window changed while maps are open for read

The window shouldn't be changed while any maps are open.

AFAICT, the intention is to perform all processing at the map's native
resolution, right? In which case ...

In main.c, Rast_set_window() should be called before the map is
opened.

Thanks much better now.

I have updated the test script

grass-addons/grass7/imagery/i.topo.corr/test_i.topo.corr_synthetic_DEM_NC.sh

accordingly.

This problem remains:

i.topo.corr input=myplane_pyr_band output=myplane_pyr_topocorr_percent
basemap=myplane_pyr_illumination zenith=45.652668 method=percent
Band myplane_pyr_band:
ERROR: Input window changed while maps are open for read

Most output of i.topo.corr looks as in GRASS 6 now.

BTW: the test_i.topo.corr_synthetic_DEM_NC.sh is a
good test for the new d.mon solution. There still seem
to be some race conditions when working with multiple
monitors.

Markus

Markus Neteler wrote:

This problem remains:

i.topo.corr input=myplane_pyr_band output=myplane_pyr_topocorr_percent
basemap=myplane_pyr_illumination zenith=45.652668 method=percent
Band myplane_pyr_band:
ERROR: Input window changed while maps are open for read

You fixed the first occurrence of G_get_set_window() in
illumination.c, in eval_c_cosi(), but missed the others, in
eval_f_cosi() and eval_d_cosi().

BTW, does it really need three separate versions of what appears to be
essentially the same function?

--
Glynn Clements <glynn@gclements.plus.com>

On Fri, Aug 12, 2011 at 9:29 AM, Glynn Clements
<glynn@gclements.plus.com> wrote:

Markus Neteler wrote:

This problem remains:

i.topo.corr input=myplane_pyr_band output=myplane_pyr_topocorr_percent
basemap=myplane_pyr_illumination zenith=45.652668 method=percent
Band myplane_pyr_band:
ERROR: Input window changed while maps are open for read

You fixed the first occurrence of G_get_set_window() in
illumination.c, in eval_c_cosi(), but missed the others, in
eval_f_cosi() and eval_d_cosi().

Fixed.

BTW, does it really need three separate versions of what appears to be
essentially the same function?

I guess not (I am not the original author). Rather likely it can be reduced
to one routine. I suppose that the DOUBLE function should survive and
the result be casted appropriately.

Markus

Markus Neteler wrote:

> BTW, does it really need three separate versions of what appears to be
> essentially the same function?

I guess not (I am not the original author). Rather likely it can be reduced
to one routine. I suppose that the DOUBLE function should survive and
the result be casted appropriately.

Either that, or use the generic functions (the ones which take a
RASTER_MAP_TYPE argument) and G_incr_void_ptr().

But looking at the i.topo.corr code, I'd suggest just using the DCELL
version. Regardless of the input type, the result is always DCELL, as
are most of the calculations.

--
Glynn Clements <glynn@gclements.plus.com>

On Sun, Aug 28, 2011 at 2:11 AM, Glynn Clements
<glynn@gclements.plus.com> wrote:

Markus Neteler wrote:

> BTW, does it really need three separate versions of what appears to be
> essentially the same function?

I guess not (I am not the original author). Rather likely it can be reduced
to one routine. I suppose that the DOUBLE function should survive and
the result be casted appropriately.

Either that, or use the generic functions (the ones which take a
RASTER_MAP_TYPE argument) and G_incr_void_ptr().

But looking at the i.topo.corr code, I'd suggest just using the DCELL
version. Regardless of the input type, the result is always DCELL, as
are most of the calculations.

Ok. Doing so, I reduced also in main.c to

       dem.rast = Rast_allocate_buf(DCELL_TYPE);
       eval_cosi(&out, &dem, zenith, azimuth);

This leads (while apparently still calculating something reasonable) to
these errors:

ERROR: Input window changed while maps are open for read
ERROR: Cairo: input file has incorrect dimensions: expected: 640x480 got:
       698x533
ERROR: Cairo: input file has incorrect dimensions: expected: 640x480 got:
       698x533

(the latter will come from the former). Any pointers?

Markus

Markus Neteler wrote:

This leads (while apparently still calculating something reasonable) to
these errors:

ERROR: Input window changed while maps are open for read

I'm fairly sure that's due to main.c:173:

173 Rast_set_window(&hd_band); /* Antes de out_open y allocate para mismo tamaƱo */

If you actually want to calculate each band at the input's native
grid, you need to open and close the base map for each band.

Also, I'm quite sure that this is wrong:

156 dem.type= Rast_open_old(base->answer, "");

It should presumably be Rast_map_type() (or use Rast_get_map_type()
instead).

ERROR: Cairo: input file has incorrect dimensions: expected: 640x480 got:
       698x533
ERROR: Cairo: input file has incorrect dimensions: expected: 640x480 got:
       698x533

I have no idea how the display driver comes into this.

--
Glynn Clements <glynn@gclements.plus.com>