-
Notifications
You must be signed in to change notification settings - Fork 7
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
Adding new Feature/timestep #87
Conversation
@zmoon Know your super busy, but maybe start a review of this next week ? Thank you! |
Starting playing with changes to NL for CI and the python/canopy_app.py script. However, still brakes the python testing as it doesn't like the new time suffixes placed on the end of the txt files it seems. @zmoon Any suggestions? |
Are the latest changes for passing CI/CD tests? |
@quaz115 Yes, but still unsuccessful, would like @zmoon help here as he put the tests in originally. Its something in the python/canopy_app.py script with my new time stamp suffixes put on the output files here. Zach please make any necessary adjustments to CI python scripts directly to make this work. Thank you! |
but not parsing the time from the file name yet
@drnimbusrain I fixed the output file recognition, but haven't dealt with possible multiple files/times yet. I am not a huge fan of having the whole time step in the file name. And
|
@zmoon Given you good suggestions, I decided to go with your option 2 for the reasons you described. Hopefully we can move this PR forward. Thanks again! |
@zmoon Can We move this forward, would like to get this capability into canopy-app soon for testing other updates. Thanks! |
@drnimbusrain how should I modify the default namelist to test the multiple timestep txt output? |
@zmoon Maybe I misunderstand your question, but you can look at how to simply run the default three timesteps for NC or TXT file in the branches updated namelist: https://github.com/noaa-oar-arl/canopy-app/blob/feature/timestep/input/namelist.canopy |
Like this commented line (seems like the default is still one time step)? canopy-app/input/namelist.canopy Line 7 in 33e9068
Was kind of hoping for point example. |
Oh, I just commented that out and reverted to single timestep to see if it would help the current CI pass for now. The line commented out is indeed the example for hopefully the new default three timestep example:
With other related NL text settings for the three times to match:
Should have explanations of these new NL time variables in the README. |
@zmoon But I see now what you are asking. I reverted back to the three times as default NL for your CI updates here, and provide an example point file for three times as well in latest commit. Let me know if this works for your changes/testing. Note I change the example point input file names as well. |
- need to specify ntime now - different canopy height for point - for cases generation, limit to single time step for now - since file_vars can be str OR list(str) now, hard to know if it is list of options or a single config only one time since CI currently overriding default nl recall squeezed before returning
6cec166
to
2a3e662
Compare
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.
@drnimbusrain I added a point example with multiple output times. I think it is fine but feel free to check.
I hope to simply the Python code for dealing with the multiple times (set file_vars
as length-1 list instead of string when single file) another time.
This PR addresses adding the capability for canopy-app to read/process/write multiple time steps, rather than only instantaneous single hour.
This adds multiple input NC and TXT files to the input/ directory for examples in the SE domain, where by default the output is put into single NC file with multiple time steps for NC file output. For TXT output, canopy-app sends the output to multiple txt files with included valid timestamp on the filename.
Here is an example for input downward shortwave flux and 2-m temperature, and output summed ISOP emissions valid @ July 01, 2022 at 11, 12, and 13 UTC:
Input Downward Shortwave Flux:
July 01 @ 11 UTC
July 01 @ 12 UTC
July 01 @ 13 UTC
Input 2-m Temperature
July 01 @ 11 UTC
July 01 @ 12 UTC
July 01 @ 13 UTC
Output Summed ISOP Biogenic Emissions
July 01 @ 11 UTC
July 01 @ 12 UTC
July 01 @ 13 UTC
@zmoon Since the new SE example files cover three time periods, I assume this will break the current CI?