Opened 4 years ago

Closed 4 years ago

Last modified 2 years ago

#2332 closed enhancement (fixed)

change r.horizon output names from "angle index" to "angle"

Reported by: zarch Owned by: grass-dev@…
Priority: normal Milestone: 7.2.0
Component: Default Version: svn-trunk
Keywords: r.horizon Cc:
CPU: All Platform: All

Description

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

  1. 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?

Change History (8)

comment:1 Changed 4 years ago by annakrat

Keywords: r.horizon added

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.

comment:2 in reply to:  1 ; Changed 4 years ago by zarch

Hi Anna,

Replying to 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!


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?

comment:3 Changed 4 years ago 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%

comment:4 in reply to:  3 Changed 4 years ago by annakrat

Replying to 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.

comment:5 in reply to:  2 ; Changed 4 years ago by wenzeslaus

Replying to zarch:

Replying to 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.

comment:6 in reply to:  5 Changed 4 years ago by zarch

Replying to wenzeslaus:

Replying to zarch:

Replying to 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: basename006_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.

comment:7 Changed 4 years ago by zarch

Resolution: fixed
Status: newclosed

Done in r61096.

comment:8 Changed 2 years ago by neteler

Milestone: 7.1.07.2.0

Milestone renamed

Note: See TracTickets for help on using tickets.