Skip to content
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

test_with_upstream_developments throwing various tantrums on different days #2492

Closed
valeriupredoi opened this issue Jul 29, 2024 · 8 comments · Fixed by #2548
Closed

test_with_upstream_developments throwing various tantrums on different days #2492

valeriupredoi opened this issue Jul 29, 2024 · 8 comments · Fixed by #2548
Assignees
Labels

Comments

@valeriupredoi
Copy link
Contributor

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:

  • we need someone to have a periodic look at it, and figure out if the fails are critical, and if so, report them (either here, or upstream at various packages); don't assume by default I am gonna do that 😁
  • maybe we should change its frequency - running nightly will mean almost invariably that we report Circle tests as FAILED on the main entry page
  • maybe we should restrict the number of dev packages we test against, and keep only the really-really important ones eg iris and netCDF4-python

I propose discuss this at the next TLT 🍺

@bouweandela
Copy link
Member

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

schlunma commented Aug 1, 2024

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
Copy link
Contributor

schlunma commented Aug 2, 2024

I implemented a draft PR that uses the latest iris release candidate here: #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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants