Closed cgallinger closed 3 years ago
scotthavens
commented
3 years ago The horizon calculation does assume an equally gridded surface. When dy != dx, as you've demonstrated, you will have to take into account the dx and dy when it's skewed.
It'd be great to have a few tests to validate the great addition. Specifically for the custom_roll, adjust_spacing and one for a surface that has a rectangular grid.
cgallinger
commented
3 years ago Hi Scott,
Thanks for the reply! I actually meant to pull these changes to my local fork of the repository, not the main one - many apologies! I’m still pretty new to GitHub, and I’ve just started trying to figure out how to integrate it with VSCode. Hopefully it won’t happen again until I have something fully-functioning ready to contribute!
The main reason I’m trying to perform this modification is to be able to work with an equirectangular lat/lon DEM, since the grid spacing is unequal away from the equator. I’m actually using this as an input to thermal models of various regions on the Moon, and it’s been delivering some very promising results so far. Since the Moon has a much smaller radius than the Earth, I’m also planning on writing modifications that take into account the higher curvature and thus drop-off of distant horizon points. This is probably not something that will be relevant for most terrestrial applications, but I’ll put it on a separate branch just in case it works and might be useful to others.
I’ll have to look into how you’ve written your unit tests to make sure these modifications are backwards-compatible and compliant with the rest of the code, but in the meantime, thanks very much for the encouragement!
All the best, -Cailin
Cailin Gallinger (she/her) Doctoral Student Department of Earth Sciences University of Western Ontario 1151 Richmond St. N. London, ON N6A 5B7 Canada
On Apr 20, 2021, at 11:32 AM, Scott Havens @.***> wrote:
The horizon calculation does assume an equally gridded surface. When dy != dx, as you've demonstrated, you will have to take into account the dx and dy when it's skewed.
It'd be great to have a few tests to validate the great addition. Specifically for the custom_roll, adjust_spacing and one for a surface that has a rectangular grid.
— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/USDA-ARS-NWRC/topocalc/pull/11#issuecomment-823370844, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHIZB7SP7EWWVMNVN6HI7ETTJWNCFANCNFSM43GVWVCA.
scotthavens
commented
3 years ago No worries.
Awesome application of the horizon. These functions were originally developed for hydrology applications so UTM and small scale was what they focused on. The original Image Processing Workbench (IPW) had some global functions that used lat/long and could account for the curvature of the Earth. I haven't used them or really gone into depth on the calculations but seems like it may help with the Moon. For example, ghorizon would take the lat/long and account for the great circle path. Could probably make it a bit more generic for the radius of the surface.
If we go down the path of taking into account the curvature of the surface, then we'll want to probably want to have a separate function that takes into account the curvature. Then there would be a clean break between a UTM like grid and a lat/lon grid.
For testing, if you run python3 -m unittest -v it will automatically test for backwards compatibility. If it fails, then we'll know that something isn't working properly.
We can get a DOI from Zenodo for this repo, we have it on many of our other ones.
Feel free to reach out outside of Github at scott.havens@usda.gov, I'm quite interested in this application!
Horizon calculation seems to be incorrect for dy < dx, with greater-than-expected horizon angles for -45° < θ < 45°, θ < -135°, θ > 135°, and lower-than-expected values for -135° < θ < -45°, 45° < θ < 135°. Probably to do with spacing adjustment.