Upload example data for AK Glacier demo; update example notebook accordingly; adjust P_rain/snow var updates #4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Grid stack is too large to store on GitHub
The gridded time series temp data is too large to store in GitHub, so for this example, implement the time series data that does NOT vary in space.
Previous syntax caused issues in cases where T was scalar/time series rather than grid. Yes, this allocates to new memory rather than overwriting the previous value in the same memory space (this should be improved later), but for user friendliness out-of-the-box, I think this is the best option for now.
madMatchstick
approved these changes
Oct 29, 2024
There was a problem hiding this comment.
@bolotinl - Thank you for the thorough and well organized PR. I was able to run the AK_Demo notebook via the "quick start" option. Just a few minor suggestions...
from osgeowas needed toimport gdal, osrintopoflow/utils/regrid.py- Recommend adding some disclosure in the Run Topoflow block mentioning runtime. It doesn't need to be overly descriptive but something along the lines of "Be aware that this simulation will run through time 525,600.0 [min] and will likely take a few hours to complete."
- This is probably obvious to those well-versed in world of
jupyter notebooksbut the user will need to first activatetf36conda environment (or similar) to run the notebook :) Probably wouldn't hurt to add that somewhere at the top.
-thanks!
10.30.2024 edit - Note I was also able to run all pre-processing steps outlined in Option 2 in the demo.ipynb
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TL;DR
P_rainandP_snowvariables to allow compatibility with both gridded and non-gridded air temp dataMore Details:
Previously, the example data and config files for running the Glacier Demo notebook was hosted on Google Drive for internal use. So that users can readily replicate this demo, I uploaded the example data and configs to the examples folder. Because the time varying grid of air temperature is too large to host on GitHub, I updated the notebook to use a time series of air temperature that does not vary in space.
As briefly considered in PR#15 to the peckhams/topoflow36 repo (but later reversed), the update of the P_rain and P_snow variables as-is causes issues if input air temperature data is not gridded, so I also made a change to the line updating these variables in
met_base.py. This solution does allocate the variable to new memory (not ideal), but for now may be the best solution to get TopoFlow running properly out-of-the-box for users following the demo notebook. Future development can help consider how we can reduce this potential inefficiency.