-
Notifications
You must be signed in to change notification settings - Fork 0
revisions of ObsFcstAna postprocessing package #111
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
revisions of ObsFcstAna postprocessing package #111
Conversation
|
@gmao-qliu, I started a new PR for the ObsFcstAna package because I forgot to ask you about a couple things before merging #87:
|
|
I've been using this package quite extensively over the last few days and it seems great for my use cases at least. |
I can see that one might want to save stats of individual species and only combine all species before plotting. I modified to enable it. |
|
@gmao-rreichle @amfox37 please review the changes made. |
…sistent variable name for endianness, vertical alignment (read_GEOSldas.py)
…_time for clarity and consistency; vertical alignment (Plot_stats_timeseries.py, Plot_stats_maps.py)
…, Plot_stats_maps.py): - removed Nmin from calc_temporal_stats_from_sums() - moved read of stats file to Plot_stats_maps.py - edited/added comments - fixed vertical alignment
gmao-rreichle
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.
@gmao-qliu : The main scripts are still rather confusing. I added a few commits and comments but ran out of time to really wrap my mind around the whole thing. In a nutshell, I think we need to clearly separate the tasks accomplished by the various "main" scripts. If we have plot functions, they should focus on plotting and avoid doing things like averaging metrics across species or applying the Nmin threshold. The latter would be better off in the main() portion of the respective scripts. Not sure if all of the comments taken together make sense. We can discuss in person tomorrow
GEOSldas_App/util/postproc/ObsFcstAna_stats/Get_ObsFcstAna_stats.py
Outdated
Show resolved
Hide resolved
|
I've completed the latest round of changes. Please review and test. @gmao-rreichle @amfox37 |
…ons; edited comments (Plot_stats_maps.py, tile_to_latlon.py)
…s (tile_to_latlongrid.py, Plot_stats_maps.py)
…proc_ObsFcstAna.py, Plot_stats_*.py)
gmao-rreichle
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.
@gmao-qliu : I made a few more changes (https://github.com/GEOS-ESM/GEOSldas_GridComp/compare/f5fa08c..b4db565?w=1; white-space changes hidden) and added a few comments below.
GEOSldas_App/util/postproc/ObsFcstAna_stats/Get_ObsFcstAna_sums.py
Outdated
Show resolved
Hide resolved
…ile_to_latlongrid.py)
Minor fixes to python package for post-processing ObsFcstAna output.
cc: @amfox37