[GRASS-dev] Update of r.surf.idw to FP

Hi,

I have tried to update r.surf.idw to floating point:
http://mpa.itc.it/markus/tmp/rsurfidw.diff

but I am not quite sure if this is efficient like this.
Currently it doesn't compile due to some "extern" magic
which I don't quite understand.
I have been inspired by r.example.

Anyone willing to take over?
thanks
markus

Markus Neteler wrote:

I have tried to update r.surf.idw to floating point:
http://mpa.itc.it/markus/tmp/rsurfidw.diff

but I am not quite sure if this is efficient like this.
Currently it doesn't compile due to some "extern" magic
which I don't quite understand.
I have been inspired by r.example.

The problem is that you have renamed some (but not all) occurrences of
"cell" to "outrast", although the variable is still called "cell".

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

On Tue, Mar 20, 2007 at 06:24:09PM +0000, Glynn Clements wrote:

Markus Neteler wrote:

> I have tried to update r.surf.idw to floating point:
> http://mpa.itc.it/markus/tmp/rsurfidw.diff
>
> but I am not quite sure if this is efficient like this.
> Currently it doesn't compile due to some "extern" magic
> which I don't quite understand.
> I have been inspired by r.example.

The problem is that you have renamed some (but not all) occurrences of
"cell" to "outrast", although the variable is still called "cell".

I see.
Thinking about it again: probably it is a waste of time.

Proposal:
- write a script to emulate r.surf.idw[2], calling
   - r.to.vect
   - v.surf.idw
- retire r.surf.idw and r.surf.idw2, substitute them by this script.

Since both r.surf.idw and r.surf.idw2 are half broken (r.surf.idw2 more
than the other) and v.surf.idw works, this might be the least cost path
for maintenance.

Markus

Markus Neteler wrote:

On Tue, Mar 20, 2007 at 06:24:09PM +0000, Glynn Clements wrote:

Markus Neteler wrote:

I have tried to update r.surf.idw to floating point:
http://mpa.itc.it/markus/tmp/rsurfidw.diff

but I am not quite sure if this is efficient like this.
Currently it doesn't compile due to some "extern" magic
which I don't quite understand.
I have been inspired by r.example.

The problem is that you have renamed some (but not all) occurrences of
"cell" to "outrast", although the variable is still called "cell".

I see.
Thinking about it again: probably it is a waste of time.

Proposal:
- write a script to emulate r.surf.idw[2], calling
   - r.to.vect
   - v.surf.idw
- retire r.surf.idw and r.surf.idw2, substitute them by this script.

Would we get rid of the script in GRASS 7? IMHO in GRASS 7 it won't be
necessary to pretend that r.surf.idw[2] is still present.

Maciek

On Tue, 20 Mar 2007, Markus Neteler wrote:

On Tue, Mar 20, 2007 at 06:24:09PM +0000, Glynn Clements wrote:

Markus Neteler wrote:

I have tried to update r.surf.idw to floating point:
http://mpa.itc.it/markus/tmp/rsurfidw.diff

but I am not quite sure if this is efficient like this.
Currently it doesn't compile due to some "extern" magic
which I don't quite understand.
I have been inspired by r.example.

The problem is that you have renamed some (but not all) occurrences of
"cell" to "outrast", although the variable is still called "cell".

I see.
Thinking about it again: probably it is a waste of time.

Proposal:
- write a script to emulate r.surf.idw[2], calling
  - r.to.vect
  - v.surf.idw
- retire r.surf.idw and r.surf.idw2, substitute them by this script.

Since both r.surf.idw and r.surf.idw2 are half broken (r.surf.idw2 more
than the other) and v.surf.idw works, this might be the least cost path
for maintenance.

From reading the manpage, it looks like r.surf.idw has some features that

aren't present in v.surf.idw. E.g. correct calculation of distances in lat/long locations, error output. However r.surf.idw2 doesn't look very useful. It seems that the functionality of r.surf.idw2 would be better kept as a command-line option for r.surf.idw. Perhaps the author of r.surf.idw had in mind that this improved version would replace r.surf.idw2, but the GRASS developers at the time were hesitant and kept it on in case anybody needed it?

Paul

On Mar 20, 2007, at 4:51 PM, Paul Kelly wrote:

On Tue, 20 Mar 2007, Markus Neteler wrote:

On Tue, Mar 20, 2007 at 06:24:09PM +0000, Glynn Clements wrote:

Markus Neteler wrote:

I have tried to update r.surf.idw to floating point:
http://mpa.itc.it/markus/tmp/rsurfidw.diff

but I am not quite sure if this is efficient like this.
Currently it doesn't compile due to some "extern" magic
which I don't quite understand.
I have been inspired by r.example.

The problem is that you have renamed some (but not all) occurrences of
"cell" to "outrast", although the variable is still called "cell".

I see.
Thinking about it again: probably it is a waste of time.

Proposal:
- write a script to emulate r.surf.idw[2], calling
  - r.to.vect
  - v.surf.idw
- retire r.surf.idw and r.surf.idw2, substitute them by this script.

Since both r.surf.idw and r.surf.idw2 are half broken (r.surf.idw2 more
than the other) and v.surf.idw works, this might be the least cost path
for maintenance.

From reading the manpage, it looks like r.surf.idw has some features that

aren't present in v.surf.idw. E.g. correct calculation of distances in lat/long locations, error output. However r.surf.idw2 doesn't look very useful. It seems that the functionality of r.surf.idw2 would be better kept as a command-line option for r.surf.idw. Perhaps the author of r.surf.idw had in mind that this improved version would replace r.surf.idw2, but the GRASS developers at the time were hesitant and kept it on in case anybody needed it?

Paul you are right - r.surf.idw2 is the original (or first) idw that was written by Mike Shapiro, a more sophisticated
version was then contributed by Greg - I actually thought that he also contributed a site version -
but apparently v.surf.idw is based on Mike's version improved by you. I think r.surf.idw2 was kept
as the "original, GRASS native" IDW as opposed to contributed r.surf.idw.

Given that r.surf.idw has built-in, working crossvalidation I would keep it and retire r.surf.idw2, but please whoever
has suitable data try it out - we should decide based on how these modules work
and whether they produce valid results (with the new data set that we are working on,
I could not getting decent results with r.surf.idw2 - it won't work even with smaller rasters,
both r.surf.idw and v.surf.idw worked better although they had some problems too.) the only issue
with r.surf.idw is that you need to convert your points to raster and by that you are introducing
a horizontal error so the results of crossvaliadtion are not as useful as if it was done
from the original points (e.g. in v.surf.idw) and the results are integer (but that can be fixed).

Helena

Paul

_______________________________________________
grass-dev mailing list
grass-dev@grass.itc.it
http://grass.itc.it/mailman/listinfo/grass-dev

Glynn Clements wrote on 03/20/2007 07:24 PM:

Markus Neteler wrote:
  

I have tried to update r.surf.idw to floating point:

but I am not quite sure if this is efficient like this.
Currently it doesn't compile due to some "extern" magic
which I don't quite understand.
I have been inspired by r.example.
    
The problem is that you have renamed some (but not all) occurrences of
"cell" to "outrast", although the variable is still called "cell".

(given that r.surf.idw should stay for more time)

I have updated the patch (find attached) to the current CVS version,
still crashing.
Advice/fix most welcome...

thanks
Markus

------------------
ITC -> dall'1 marzo 2007 Fondazione Bruno Kessler
ITC -> since 1 March 2007 Fondazione Bruno Kessler
------------------

(attachments)

r_surf_idw_fp.patch.gz (2.59 KB)

On Wed, Sep 05, 2007 at 02:54:57PM +0200, Markus Neteler wrote:

Glynn Clements wrote on 03/20/2007 07:24 PM:
> Markus Neteler wrote:
>
>> I have tried to update r.surf.idw to floating point:
>>
>> but I am not quite sure if this is efficient like this.
>> Currently it doesn't compile due to some "extern" magic
>> which I don't quite understand.
>> I have been inspired by r.example.
>>
>
> The problem is that you have renamed some (but not all) occurrences of
> "cell" to "outrast", although the variable is still called "cell".
>
>
(given that r.surf.idw should stay for more time)

I have updated the patch (find attached) to the current CVS version,
still crashing.
Advice/fix most welcome...

Still struggling with the FP patch (find attached).

Markus

------------------
ITC -> dall'1 marzo 2007 Fondazione Bruno Kessler
ITC -> since 1 March 2007 Fondazione Bruno Kessler
------------------

(attachments)

r_surf_idw_fp.patch (9.05 KB)

Markus Neteler wrote:

> >> I have tried to update r.surf.idw to floating point:
> >>
> >> but I am not quite sure if this is efficient like this.
> >> Currently it doesn't compile due to some "extern" magic
> >> which I don't quite understand.
> >> I have been inspired by r.example.
> >>
> >
> > The problem is that you have renamed some (but not all) occurrences of
> > "cell" to "outrast", although the variable is still called "cell".
> >
> >
> (given that r.surf.idw should stay for more time)
>
> I have updated the patch (find attached) to the current CVS version,
> still crashing.
> Advice/fix most welcome...

Still struggling with the FP patch (find attached).

1. A function cannot reference local variables of another function.
The outrast variable needs to either be a global variable (like cell)
or passed as a parameter.

As it stands, the outrast declared in main() and the outrast
referenced by interpolate() and make_neighbors_list() are two
different variables (the first is local, the second global). Only the
former is actually getting initialised, hence the segfault.

2. Changing the type of the cell argument to row_lists() from CELL *
to unsigned char * stops the compiler complaining, but the compiler is
right to complain. If cell can refer to any of CELL, FCELL or DCELL,
any code which dereferences CELL needs three cases.

But those values are stored in the value field in the MELEMENT
structure, which is an int. So this field qwould need to be either a
double or a union of CELL, FCELL, and DCELL, and any code which
references that field would also need three cases.

Personally, I would suggest simply replacing CELL with DCELL
throughout, rather than trying to write three separate cases for
everything. If you want to be able to generate CELL output maps (it
doesn't necessarily follow that you want CELL out just because you got
CELL in), just open the output map as the appropriate type.

Most of the time, having separate CELL/FCELL/DCELL cases isn't worth
the effort. Sometimes it's worth providing special treatment for
integers, but not when you're perfoming interpolation (which naturally
produces a FP result even if the inputs are integers).

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