ESMValGroup / ESMValCore

ESMValCore: A community tool for pre-processing data from Earth system models in CMIP and running analysis scripts.
https://www.esmvaltool.org
Apache License 2.0
42 stars 38 forks source link

`test_with_upstream_developments` throwing various tantrums on different days #2492

Open valeriupredoi opened 1 month ago

valeriupredoi commented 1 month ago

As we can see from Circle's test entries on main, this test is rather temperamental (and it's OK and normal to be this way, after all, we are testing bleeding edges of multiple packages). What concerns me is this, though:

I propose discuss this at the next TLT :beer:

bouweandela commented 1 month ago

Most failures appear in the ICON fixes and are caused by changes in how Iris handles meshes in the upcoming 3.10 release: https://github.com/SciTools/iris/releases/tag/v3.10.0rc0. Maybe @schlunma could have a look at these?

schlunma commented 1 month ago

I will do that once Iris 3.10 is out. Some of the fails are also related to iris-esmf-regrid, so we also have to wait until they fixed their mesh handling.

valeriupredoi commented 1 month ago

thank you lots, gents! We still need a structured approach to flagging/dealing with these types of fails. Let's chat about it in person (2d-person) on our next week's TLT (we may have to postpone it if peeps are on holidays)

schlunma commented 1 month ago

Update: iris-esmf-regrid just published a new release where they address iris' new API in a nice backwards-compatible way: https://github.com/SciTools-incubator/iris-esmf-regrid/releases/tag/v0.11.0. We should do the same 👍

schlunma commented 1 month ago

I implemented a draft PR that uses the latest iris release candidate here: https://github.com/ESMValGroup/ESMValCore/pull/2500. Everything seems to work fine, also the test_with_upstream_developments run successfully now 👍

EDIT: I did not implement it in a backwards-compatible way but rather pinned iris so we can use its new features (and get rid of 300+ lines!)

valeriupredoi commented 1 month ago

I like that, and also commented there that'd I'd be very happy to review it, many thanks, Manu! About being strictly incompatible with older than latest irises, I am in two boats about it - what if the shiniest latest iris proves out to have a major bug, and we need to wait until they fix it and release a patched, new version - we using the buggy iris since we can't revert to an older one - may take would be to have support for at least one lesser iris version - @bouweandela (hope you back from :beach: ) what you think?

bouweandela commented 1 month ago

I agree that it would be nice to support multiple versions of iris, but that may be a luxury we cannot afford at the moment. Let's talk about it at the next tech lead team meeting.

valeriupredoi commented 1 month ago

good call, bud! Another point about this test (before I forget it): we should prob not run it in PRs - people get very confused why their stuff is failing, run it on main only