-
Notifications
You must be signed in to change notification settings - Fork 25
Switch map elevations to OpenElevationAPI #3688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Takes GoogleElevationAPI off our bill completely. Pretty nifty coding from Claude, too. Very minimal changes to Stimulus; the call comes from Ruby. New files: - app/controllers/concerns/cors_headers.rb - CORS concern - app/controllers/geo/base_controller.rb - Base geo controller - app/controllers/geo/elevations_controller.rb - Elevation proxy - test/controllers/geo/elevations_controller_test.rb - Tests Modified files: - app/controllers/api2_controller.rb - Uses CORS concern - app/javascript/controllers/geocode_controller.js - Uses /geo/elevation - app/javascript/controllers/map_controller.js - Input fallback for sampleElevationPoints - config/routes.rb - Added /geo/elevation route - test/system/location_form_system_test.rb - Added (skipped) elevation test - test/test_helper.rb - Allow Open-Elevation & Google APIs
mo-nathan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this seems to be working great! However, I noticed that the "Get Elevations" button is now seems to always be disabled (or I don't know how to enable it). If it is always disabled, then we should get rid of it. Even if it's only very often disabled, then I think it would be better to hide it rather than disable it.
|
I found the source of our Google Elevation API billing issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job. Code works as intended.
~~ But how reliable/accurate is Open-Elevation?~~
How are we determining min/max?
AFAIK both Open-Elevation and Google can return elevation only for points.
So min/max for boxes can be way off.
I tried this:
- Edit Burkina Faso http://localhost:3000/locations/17748/edit
- Get Elevations
- Result:
- Highest Elevation: 344
- Lowest Elevation: 260
The highest point in Burkina Faso is Mount Tenakourou, at 747 m.
https://www.cia.gov/the-world-factbook/countries/burkina-faso/
Google Maps terrain view
So min/max using Open-Elevation was off by > 1,300 feet.
For Mount Hood National Forest, it gave me 17/1096. The high point there is Mount Hood, 3,429m. So we're off by 1.45 miles.
|
@JoeCohen Great question and I appreciate your thorough documentation of the issue. Claude says: Better sampling with Open-Elevation Since you're already using https://open-elevation.com/ (1,000 free requests/month), you could increase the sample density:
Open-Elevation accepts multiple points per request, so a 7×7 grid is still just 1 API call. Recommendation: A 5×5 or 7×7 grid would be a good balance - much better coverage for mountainous terrain while staying within a single request. Want me to implement that? I said: How about 9x9! |
|
Switched to 9x9 and getting better elevations. Mount Hood National Forest now gives 2073/163m, which is only off by a mile ;) "Still, she persevered." She proposed a test of increasing grid density, and here's what she got. ⏺ Results for Mt. Hood area:
Mt. Hood's actual peak is 3,429m - so even 21×21 (441 points) only captured 79% of the true elevation. But 21×21 gets us 630m higher than 9×9. The higher density clearly helps for mountainous terrain. The lows are also interesting: 9x9 seems closest to the true low elevation. That's the classic grid sampling problem. The 9×9 grid happened to land on a lower point that the denser grids missed entirely. The "true" low for that area (which includes parts of the Columbia River valley) could be close to sea level. What grid size do you want to use? 12×12 (144 points) might be a good balance - still a single API call, reasonable accuracy. Or go bigger if the API can handle it. Re: button, modified Get Elevations button to be hidden instead of disabled after fetch. Claude says: ⏺ The elevations are auto-fetched when:
The "Get Elevations" button is only needed for the edge case where someone manually types NSEW coordinates directly into the input fields without any map/geocode interaction. So the current behavior (button visible initially, hidden after fetch) should work well - it's there if you need it for manual entry, and disappears once elevations are populated. |
|
Pursuant to the conversation on Slack, i will shelve this PR. |
Fixes the broken "Get Elevations" button on the Location form - switches from Google Elevation API (which has referrer restrictions) to Open-Elevation API. Elevations auto-populate after geocoding a location. Fixes #3690.
Takes GoogleElevationAPI off our bill completely. Pretty nifty coding from Claude, too. Very minimal changes to Stimulus; the call comes from Ruby.
Manual Test
There's a new system test for the elevation fields getting populated from OpenElevation, but manual test welcome. Pick a place to geocode in the /locations/new form, and it should get its bounds and elevation min/max populated.
Changes
New files:
Modified files: