[GRASS-dev] [GRASS GIS] #2332: change r.horizon output names from "angle index" to "angle"

#2332: change r.horizon output names from "angle index" to "angle"
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: normal | Milestone: 7.1.0
Component: Default | Version: svn-trunk
Keywords: | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------
At the moment r.horizon combine the prefix with the angle index.

I think that use the angle degree instead of the angle index, could be
clearer and easier to interpret the raster name. Moreover using the angler
can help to avoid situations, like: imagine that you generate all the map
using a horizon step of 10 degrees,

{{{
$ r.horizon elev_in=elevation horizon=h_ hrorizon_step=10
}}}

now suppose that, after some testing you want to use a step of 5 degrees,
with the current approach user have to run setting horizon_step to 5...
Recomputing half of the already existing maps.

If the parameters horizon_start and horizon_end will be accepted
user can just compute the missing raster maps with:

{{{
$ r.horizon elev_in=elevation horizon=h_ hrorizon_step=10 horizon_start=5
}}}

And here raise the name problem, using the angle index convention we are
going to overwrite all the previous raster map, so user should change the
horizon parameters like:

{{{
$ r.horizon elev_in=elevation horizon=h2_ hrorizon_step=10 horizon_start=5
}}}

And then write a script to "merge" the outputs renaming the raster maps
accordingly with index convention. If we change from the angle index to
the angle we solve the name conflicts, generating (h0, h5, h10, etc)
instead of (h0, h1, h3, etc).

Change from the angle index (integer) to the angle (double) we have to
manage some how the decimals. I see two main options:

1. add an extra parameter with the number of decimals to use in the format
name.

   {{{
$ r.horizon elev_in=elevation horizon=h_ hrorizon_step=0.5 horizon_end=2
decimals=1
}}}

   It will generate: h_0.0, h_0.5, h_1.0, h_1.5

2. change the horizon parameter from raster prefix to the format string,
so the command will be:

   {{{
$ r.horizon elev_in=elevation horizon=h_%4.1f hrorizon_step=0.5
horizon_end=2
}}}

   It will generate: h_0.0, h_0.5, h_1.0, h_1.5

On the other hand change the r.horizon convention it means that we are
going to break others modules that are using r.horizon output maps as
input such as r.sun, r.sky view, etc. but should not too dificult to fix.

What do you think?

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

#2332: change r.horizon output names from "angle index" to "angle"
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: normal | Milestone: 7.1.0
Component: Default | Version: svn-trunk
Keywords: r.horizon | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------
Changes (by annakrat):

  * keywords: => r.horizon

Comment:

Could we set the number of digits automatically based on the number of
decimal digits of the step and number of digits of the maximum angle? For
example when horizon_step is 0.5 and maximum angle is 50, the maps would
be named h_06.5, h_07.0, h_07.5 and so on. I don't like the dot there,
though, since it's not valid for vector maps and that's a source of
confusion then.

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

#2332: change r.horizon output names from "angle index" to "angle"
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: normal | Milestone: 7.1.0
Component: Default | Version: svn-trunk
Keywords: r.horizon | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------

Comment(by zarch):

Hi Anna,

Replying to [comment:1 annakrat]:
> Could we set the number of digits automatically based
> on the number of decimal digits of the step and number
> of digits of the maximum angle? For example when
> horizon_step is 0.5 and maximum angle is 50, the maps
> would be named h_06.5, h_07.0, h_07.5 and so on.

I really like your idea!

[[BR]]

> I don't like the dot there, though, since it's not valid for
> vector maps and that's a source of confusion then.

I don't like either, but I don't see any better solution, do you?

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

#2332: change r.horizon output names from "angle index" to "angle"
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: normal | Milestone: 7.1.0
Component: Default | Version: svn-trunk
Keywords: r.horizon | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------

Comment(by zarch):

ok, the code is almost ready, this is how it behaves:

{{{
$ r.horizon elevation=elevation step=0.05 end=0.2 basename=rmvm --o
Calculating map 1 of 10 (angle 0.00, raster map <rmvm_000.00>)
  100%
Calculating map 2 of 10 (angle 0.05, raster map <rmvm_000.05>)
  100%
Calculating map 3 of 10 (angle 0.10, raster map <rmvm_000.10>)
  100%
Calculating map 4 of 10 (angle 0.15, raster map <rmvm_000.15>)
  100%

$ r.horizon elevation=elevation step=0.5 end=2 basename=rmvm --o
Calculating map 1 of 4 (angle 0.00, raster map <rmvm_000.0>)
  100%
Calculating map 2 of 4 (angle 0.50, raster map <rmvm_000.5>)
  100%
Calculating map 3 of 4 (angle 1.00, raster map <rmvm_001.0>)
  100%
Calculating map 4 of 4 (angle 1.50, raster map <rmvm_001.5>)
  100%

$ r.horizon elevation=elevation step=5 end=20 basename=rmvm --o
Calculating map 1 of 4 (angle 0.00, raster map <rmvm_000>)
  100%
Calculating map 2 of 4 (angle 5.00, raster map <rmvm_005>)
  100%
Calculating map 3 of 4 (angle 10.00, raster map <rmvm_010>)
  100%
Calculating map 4 of 4 (angle 15.00, raster map <rmvm_015>)
  100%
}}}

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

#2332: change r.horizon output names from "angle index" to "angle"
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: normal | Milestone: 7.1.0
Component: Default | Version: svn-trunk
Keywords: r.horizon | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------

Comment(by annakrat):

Replying to [comment:3 zarch]:
> ok, the code is almost ready, this is how it behaves:
>
That looks great, the only thing I am not sure about is the end parameter
which is not included. That might be confusing for the user because it's
more programming approach than what users are used to. But I agree with it
as far as it is mentioned in the label/description of the parameter.

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

#2332: change r.horizon output names from "angle index" to "angle"
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: normal | Milestone: 7.1.0
Component: Default | Version: svn-trunk
Keywords: r.horizon | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------

Comment(by wenzeslaus):

Replying to [comment:2 zarch]:
> Replying to [comment:1 annakrat]:
> > Could we set the number of digits automatically based
> > on the number of decimal digits of the step and number
> > of digits of the maximum angle? For example when
> > horizon_step is 0.5 and maximum angle is 50, the maps
> > would be named h_06.5, h_07.0, h_07.5 and so on.

> > I don't like the dot there, though, since it's not valid for
> > vector maps and that's a source of confusion then.
>
> I don't like either, but I don't see any better solution, do you?

The only possibility I see is to replace this dot with an underscore, too:
h_06_5, h_07_0, h_07_5. But the names are not so nice anymore but wouldn't
use colon for hour neither (h_06:30 vs h_06_30), so maybe the underscore
is actually quite good.

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

#2332: change r.horizon output names from "angle index" to "angle"
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: normal | Milestone: 7.1.0
Component: Default | Version: svn-trunk
Keywords: r.horizon | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------

Comment(by zarch):

Replying to [comment:5 wenzeslaus]:
> Replying to [comment:2 zarch]:
> > Replying to [comment:1 annakrat]:
> > > I don't like the dot there, though, since it's not valid for
> > > vector maps and that's a source of confusion then.
> >
> > I don't like either, but I don't see any better solution, do you?
>
> The only possibility I see is to replace this dot with an
> underscore, too: h_06_5, h_07_0, h_07_5. But the names are
> not so nice anymore but wouldn't use colon for hour neither
> (h_06:30 vs h_06_30), so maybe the underscore is actually
> quite good.

Actually I don't like too much the idea of replace dot with underscore, we
loose readability. To improve readability perhaps we can use two
underscore as separator between basename and numbers, like:
basename__006_5

The other disadvantage is that instead of using the standard C format
string like: "basename_%05.2f" to generate the name, we have to define a
function to split the integer and the decimal part, and we have to take
care if the number of decimals is 0, then we have to switch the format,
something like:

{{{
void get_name(const char *basename, double number, size_t decimals, char
*name)
{
     double integer, decimal;
     integer = floor(number);
     if (decimals != 0){
         decimal = ((number - integer) * pow(10., (double)decimals));
         sprintf(name, "%s__%03d_%d", basename, (int)integer,
(int)decimal);
     }
     else{
         sprintf(name, "%s__%03d", basename, (int)integer);
     }
}
}}}

and then:

{{{
get_name("basename", 12.34567890, 2, name); // => basename__012_34
get_name("basename", 12.34567890, 0, name); // => basename__012
}}}

If we choose this option, this function should be put somewhere in GRASS
library because other modules like r.sun must be able to reproduce the
same logic, moreover have a function that generate the name given the
basename and number should make easier to maintain consistency over the
grass modules.

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

#2332: change r.horizon output names from "angle index" to "angle"
--------------------------+-------------------------------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: enhancement | Status: closed
  Priority: normal | Milestone: 7.1.0
Component: Default | Version: svn-trunk
Resolution: fixed | Keywords: r.horizon
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Changes (by zarch):

  * status: new => closed
  * resolution: => fixed

Comment:

Done in [http://trac.osgeo.org/grass/changeset/61096 r61096].

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