Add "models" URL filter to CONUS hydrology endpoint#702
Add "models" URL filter to CONUS hydrology endpoint#702
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an optional models query parameter to CONUS hydrology endpoints to filter response payloads by selected model names.
Changes:
- Documented the new
modelsURL parameter for/conus_hydrology/statsand/conus_hydrology/modeled_climatology. - Implemented model filtering and validation via a shared request-args helper.
- Expanded model validation to accept both CMIP6 downscaled models and legacy Hydroviz models.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| templates/documentation/conus_hydrology.html | Documents ?models=... usage and chaining with other params. |
| routes/conus_hydrology.py | Parses models from query string and filters packaged outputs accordingly. |
| luts.py | Adds a curated Hydroviz model allowlist and unions it into all_possible_models. |
| application.py | Updates request validation to accept the expanded model allowlist. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Joshdpaul
left a comment
There was a problem hiding this comment.
Excellent work! This is a great solution - no need to involve custom WCS requests for this coverage. This is an interesting (and maybe unique?) case in which we are limiting the request not because of the time it takes for the request to be performed, but the size of the JSON result we get back.
I tested all endpoints with valid + bogus model names, and also tested CSV outputs. Looks like everything works as it should! 5/5 stars ⭐ ⭐ ⭐ ⭐ ⭐
|
Thanks for the review on this, @Joshdpaul, and sorry this ended up being the wrong path forward. This PR has been replaced with #703. I'm going to close this PR without merging. |
Xref: ua-snap/hydroviz#98
Xref: ua-snap/hydroviz#86
This PR adds an optional "models" URL parameter to filter the output of the
/conus_hydrology/statsand/conus_hydrology/modeled_climatologyendpoints.This is implemented in a simple, crude, but performant way. After fetching data for all models from Rasdaman, this implementation simply deletes the models that weren't requested after-the-fact. I spent some time exploring what it would take to implement this using Rasdaman axis subsetting and it requires a lot of code in a lot of places, lots of opportunities for new bugs, and has very little performance benefit. I benchmarked my partial solution using Rasdaman axis subsetting and it actually took more time than just requesting all the models and lopping off branches afterwards. This is because requesting data for multiple models that are noncontiguous along the model axis (e.g., Maurer and CCSM4) require multiple WCS requests for each endpoint, whereas grabbing all of the model data generally takes less than 1 second per endpoint.
So, this seems like a decent solution and reduces the API data downloaded by the hydroviz webapp to about 10% of what it previously was for the Maurer & CCSM4 models I've tested with, from about 10mb of data to 1mb.
To test, try URLs like the following examples and confirm that only your requested models show up in the response:
http://localhost:5000/conus_hydrology/stats/27226?models=Maurer,CCSM4
http://localhost:5000/conus_hydrology/modeled_climatology/27226?models=Maurer,CCSM4
Also try adding bogus models to the URL to make sure validation fails.
You can also test this PR by pointing the corresponding
hydroviz_model_filterbranch in the hydroviz webapp PR against this branch: ua-snap/hydroviz#105I've updated the documentation for the
/conus_hydrology/statsand/conus_hydrology/modeled_climatologyendpoints to mention the new "models" parameter. I've also configured the/conus_hydrology/observed_climatologyendpoint to return an "Invalid GET paramater" error page if you try to filter by model, since it does not support this feature.