ailich / MultiscaleDTM

Calculates multi-scale geomorphometric terrain attributes from regularly gridded DEM/bathymetry rasters.
https://ailich.github.io/MultiscaleDTM/
GNU General Public License v3.0
21 stars 4 forks source link

Add default radius for RelPos() annulus and clarify behavior. #66

Closed benjaminmisiuk closed 1 year ago

benjaminmisiuk commented 1 year ago

There is currently a default radius for circular focal windows but not annulus in RelPos(). This may be intentional, but you could consider adding or describing why there is no default. Additionally, it's my understanding that when radius is specified as a vector of length 1 for an annulus the inner and outer radii both use this value. Consider mentioning that in the documentation.

ailich commented 1 year ago

Thanks @benjaminmisiuk, The reason for the default for circular is I feel like at least if working in cell units, immediate neighbors is an obvious choice for a circle whereas for annulus it's less clear cut (e.g. should it be c(1,2) or c(1,1)). As for your second point I'm going to remove the ability to specify a 1 length radius for annulus. I think for BPI it was fine since it needed an annulus window, but with RelPos I think it can lead to confusion.

benjaminmisiuk commented 1 year ago

Sounds good on both accounts - thanks.