-
Notifications
You must be signed in to change notification settings - Fork 128
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
Generate climatology on the fly for AutoAssess soil moisture #3197
Conversation
Required in order to access preprocessed obs via main esmvaltool config, and in any case our strategy is now to port autoassess metrics as "native" esmvaltool code. Component changes: - name diag_script directly in recipe - add main() function to handle config and extract filenames - update land_sm_top() to use these preprocessed files
@valeriupredoi don't drop your pint 🍺, but I finally made good progress on fixing #2309. I wonder if you'd be interested in tech reviewing it? |
Science impactThis will need science review, and I am discussing with relevant experts at the Met Office (though others welcome also to review if interested). There are two science questions regarding the change in results:
Attached (soil_moisture_clims.pdf) is a comparison addressing the first question, the conclusion of which is that the original and new on-the-fly climatologies are consistent in the regions for which they both have coverage. |
To check the second science question, I tested the impact of changing the climatology alone. This was done by running the original recipe using the main branch at ESMValTool 2.8.0 (so no changes to the diag script), and pointing the recipe at the climatology produced by the ESMValTool pre-processor in runs of this PR. The results are as follows:
Thus the changes come from the climatology. |
@valeriupredoi I now see the Codacy errors - I'll fix those next week. |
Last one requires a minor refactor
Fixes problem of too-many-locals (Alternative approach would be to find another pub?)
...since diag_script is called directly
OK, that's the Codacy errors fixed @valeriupredoi, and also documentation updated, so I think ready for technical review. Would you be up for reviewing this one? (Am still discussing science change with Graham Weedon at the Met Office. He agrees with my analysis so far, though has asked for some more plots/numbers to fully evidence up my assertion.) |
Have concluded science review discussion with Graham Weedon at MetO, having produced the additional plots/stats he suggested. See new doc for plots and my interpretation: soil_moisture_clims.pdf
|
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.
Great to see these improvements @alistairsellar! From the code, it looks like provenance is not written for some of the output files. Could you add it for all files please? See https://docs.esmvaltool.org/en/latest/community/diagnostic.html#recording-provenance for more information.
esmvaltool/recipes/recipe_autoassess_landsurface_soilmoisture.yml
Outdated
Show resolved
Hide resolved
@esmvalbot Please run recipe_autoassess_landsurface_soilmoisture.yml |
Since @bouweandela asked, ESMValBot will run recipe recipe_autoassess_landsurface_soilmoisture.yml as soon as possible, output will be generated here |
It looks like the recipe documentation page could also use some updates and formatting improvements. E.g. I see a bulleted list that is malformatted into a single line and mentions of the script |
…oup/ESMValTool into autoassess_soilmoisture_obs_fly
Provenance now sorted, for both the metrics files in first diag_script, and also the general AutoAssess metrics plotter. So I think that addresses all of @bouweandela's requests, apart from the citation warnings, which will be fixed by ESMValGroup/ESMValCore#2097. @valeriupredoi ready for final check and merge, pending merge of ESMValGroup/ESMValCore#2097. |
@esmvalbot Please run recipe_autoassess_landsurface_soilmoisture.yml |
Since @bouweandela asked, ESMValBot will run recipe recipe_autoassess_landsurface_soilmoisture.yml as soon as possible, output will be generated here |
ESMValBot is happy to report recipe recipe_autoassess_landsurface_soilmoisture.yml ran OK, output has been generated here |
esmvaltool/diag_scripts/autoassess/land_surface_soilmoisture/soilmoisture.py
Outdated
Show resolved
Hide resolved
@esmvalbot Please run recipe_autoassess_landsurface_soilmoisture.yml |
Since @bouweandela asked, ESMValBot will run recipe recipe_autoassess_landsurface_soilmoisture.yml as soon as possible, output will be generated here |
ESMValBot is happy to report recipe recipe_autoassess_landsurface_soilmoisture.yml ran OK, output has been generated here |
esmvaltool/diag_scripts/autoassess/land_surface_soilmoisture/soilmoisture.py
Outdated
Show resolved
Hide resolved
esmvaltool/diag_scripts/autoassess/land_surface_soilmoisture/soilmoisture.py
Outdated
Show resolved
Hide resolved
@esmvalbot Please run recipe_autoassess_landsurface_soilmoisture.yml |
Since @alistairsellar asked, ESMValBot will run recipe recipe_autoassess_landsurface_soilmoisture.yml as soon as possible, output will be generated here |
ESMValBot is happy to report recipe recipe_autoassess_landsurface_soilmoisture.yml ran OK, output has been generated here |
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.
very cool, guys, let's get this in 2.9 - many thanks for fixing this, a long standing issue on my to-do list 😁 🍺
rumbold_heather: | ||
name: Heather, Rumbold | ||
institute: Met Office, UK | ||
orcid: |
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.
have you asked Heather? I can ask Steve on Whatsapp to ask her, if not 😁
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.
According to ORCID website she does not have one. She is on maternity leave at present.
Thanks for your help @bouweandela and @valeriupredoi! |
…old_and_clone_task_rtw * recipe_test_workflow_prototype: (199 commits) #3169: Upgrade the RTW to work with ESMValTool v2.8.0 [Condalock] Update Linux condalock file (#3237) Modified links to the tutorial (#3236) Add ESMValCore release `v2.8.1` into the documentation (#3235) Generate climatology on the fly for AutoAssess soil moisture (#3197) New recipe and diagnostic for Arctic-midlatitude research (#3021) Fixed pandas diagnostics for pandas>=2.0.0 (#3209) Update obs4MIPs dataset to the current naming scheme in recipe_smpi.yml (#2991) Add variable long names to provenance record in monitoring diagnostics (#3222) Extension of NASA MERRA2 CMORizer (cl, cli, clivi, clw, clwvi) (#3167) Remove "fx_variable" from recipe_wenzel14jgr.yml (#3212) [Condalock] Update Linux condalock file (#3217) Add Seaborn diagnostic (#3155) Remove fx_variables from ipccwg1ar5ch9 recipes (#3215) Remove "fx_variable" from recipe_tebaldi21esd.yml (#3211) Update recipe_impact.yml to work with newer versions of `pandas` (#3220) Use ESMValCore v2.9.0 release candidates (#3219) [Github Actions ] Check if python minor version changed after Julia install in development installation test (#3213) New plot_type 1d_profile in monitor (#3178) Add support for using a dask distributed scheduler (#3151) ...
Co-authored-by: Bouwe Andela <b.andela@esciencecenter.nl>
Description
Currently the AutoAssess soil moisture recipe (
recipe_autoassess_landsurface_soilmoisture.yml
) only runs on Jasmin because it requires a climatology which is not available on other machines. This PR makes the recipe portable by generating the climatology on the fly from Tier 2 ESA-CCI data.In this branch, the climatology is instead made using the ESMValTool seasonal pre-processor. Accessing the pre-processed data requires the script to have direct access to the ESMValTool configuration dictionary, and therefore the soil moisture diag script no longer uses the AutoAssess wrapper script (which translates config information to an alternative dictionary). This makes the recipe a more typical ESMValTool recipe, which in any case aligns with the wider strategy for porting AutoAssess.
The climatology produced by the ESMValTool preprocessor has much greater spatial coverage (fewer points masked out) than the original climatology. At points common to both climatologies the values are very similar but the change in coverage does impact on the metrics, because different regions have different errors. More detail in the comments below to verify that the changes in results are due to the coverage change only. This will need science review, and I am discussing with relevant experts at the Met Office (though others welcome also to review if interested).
Before you get started
Checklist
It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.
New or updated recipe/diagnostic
To help with the number of pull requests: