ESMValGroup / ESMValTool

ESMValTool: A community diagnostic and performance metrics tool for routine evaluation of Earth system models in CMIP
https://www.esmvaltool.org
Apache License 2.0
217 stars 127 forks source link

Add recipe for sea ice area and extents in southern polar region #3607

Open flicj191 opened 4 months ago

flicj191 commented 4 months ago

Description

New recipe for sea ice area trends and extents around Antarctica. Using code from @anton-seaice for a COSIMA (Consortium for Ocean-Sea Ice Modelling in Australia) 'cookbook' example.

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:

flicj191 commented 4 months ago

@rbeucher see draft pr for sea ice recipe

flicj191 commented 3 months ago

this recipe could possibly be moved to examples ?

flicj191 commented 3 months ago

@headmetal @rhaegar325 can you also have a look at this when you get a chance?

bouweandela commented 3 months ago

@flicj191 Could you please have a look at the checklist and make sure you've addressed all the items as best as you can? Our convention is that the author of the pull request fills in the checklist before asking for a review. If anything is unclear don't hesitate to ask.

flicj191 commented 3 months ago

Codacy issues were too many local variables and a docstring that matches PEP257 from what I can see. Are these acceptable? @ESMValGroup/esmvaltool-developmentteam

bouweandela commented 3 months ago

Yes, a few issues should be fine.

bouweandela commented 2 months ago

the input data file type is a mystery to me - why use xarray here?

The choice for xarray here seems mostly a personal preference and that is absolutely fine!

However, re-implementing existing preprocessor functions in the diagnostic is something that should preferably be avoided.

valeriupredoi commented 2 months ago

The choice for xarray here seems mostly a personal preference and that is absolutely fine!

In general, I tend to agree with this, but this may lead to a number of issues:

That's why I asked Felicity, if possible, to stick with the oldschool Iris way :beer:

rbeucher commented 2 months ago

I agree with @valeriupredoi and @bouweandela that the preprocessors should handle the heavy lifting. Since they're based on IRIS, we're somewhat limited at the moment. Supporting XArray in the future would be great, though I understand it's a complex task.

bouweandela commented 2 months ago

@rbeucher Please open a GitHub issue if you encounter such limitations. We are applying for funding for future work at the moment, so if we are aware of shortcomings in the preprocessor, we can plan to solve these issues in future projects.

rbeucher commented 2 months ago

Sure, I will. We haven't worked on this recently. Hopefully soon.

alistairsellar commented 1 month ago

Hi @flicj191 if you are still looking for a science reviewer I could ask one of the sea-ice team at the Met Office. I would think that @anton-seaice's review would suffice for science review since he knows what the outputs should look like. But let me know if you think it really needs another science reviewer.

flicj191 commented 1 month ago

Hi @flicj191 if you are still looking for a science reviewer I could ask one of the sea-ice team at the Met Office. I would think that @anton-seaice's review would suffice for science review since he knows what the outputs should look like. But let me know if you think it really needs another science reviewer.

Thanks @alistairsellar ! I will do a bit more work on this to test some regridders then will get @anton-seaice to have a look at the outputs so sounds good for that to be the science review. Cheers

alistairsellar commented 1 month ago

Thanks @alistairsellar ! I will do a bit more work on this to test some regridders then will get @anton-seaice to have a look at the outputs so sounds good for that to be the science review. Cheers

Sounds good @flicj191 !

flicj191 commented 3 weeks ago

Hi @bouweandela @valeriupredoi I have restructured the recipe to use the preprocessors ready for re-review

Checking some different regridding preprocessors, This is a plot from previous diagnostic script map_difference png with these packages on the main branch I believe:

INFO    [3335387] ESMValCore: 2.11.0.dev8+gb47f7c2ca
INFO    [3335387] ESMValTool: 2.10.0.dev68+gf16c7eab7.d20231122

then with this preprocessor:

  map_extents: &map
    climate_statistics:
      operator: mean
      period: monthly
    extract_region: 
      start_longitude: 0
      end_longitude: 360
      start_latitude: -90
      end_latitude: -50
    regrid:
      target_grid: 0.5x0.5
      scheme: linear

siconc_linear

  map_esmpy:
    <<: *map
    regrid:
      target_grid: 0.5x0.5
      scheme:
        reference: esmf_regrid.schemes:ESMFBilinear

siconc_esmpy then the target_grid to observations seems to do something strange:

  map_target:
    <<: *map
    regrid:
      target_grid: NSIDC-G02202-sh
      scheme: linear

siconc_target

  map_esmpy_target:
    <<: *map
    regrid:
      target_grid: NSIDC-G02202-sh
      scheme:
        reference: esmf_regrid.schemes:ESMFBilinear
        mdtol: 0.7

siconc_esmpy_target I'll have a look at the preprocessed files when I can. @anton-seaice, not sure if there's a specific regridding scheme you would need?

anton-seaice commented 3 weeks ago

I expect bilinear interpretation is a good choice. The monthly averages of sea ice concentration should be fairly 'smooth' in space, so biliear is appropriate.