Skip to content

Conversation

@valeriupredoi
Copy link
Contributor

@valeriupredoi valeriupredoi commented Nov 21, 2022

Description

Let's see if indeed SegFaults that we used to see are now gone (bar, of course, the ones - many of them - from netCDF>1.6.0 + iris). Don't merge this just yet (hence draft, I'd like to run a few of these test sets).


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.


To help with the number pull requests:

@codecov
Copy link

codecov bot commented Nov 21, 2022

Codecov Report

Merging #1819 (eb0043a) into main (5644203) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1819   +/-   ##
=======================================
  Coverage   91.50%   91.50%           
=======================================
  Files         202      202           
  Lines       10919    10919           
=======================================
  Hits         9991     9991           
  Misses        928      928           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@valeriupredoi
Copy link
Contributor Author

valeriupredoi commented Nov 21, 2022

Test batches (Circle + GA: test and monitor): 7 since commit 66c8c99 - that's a lot of tests with no SegFault 🥳

@valeriupredoi valeriupredoi marked this pull request as ready for review November 22, 2022 12:53
Co-authored-by: Bouwe Andela <b.andela@esciencecenter.nl>
@bouweandela
Copy link
Member

You also need to remove the sequential marker from setup.cfg

@valeriupredoi
Copy link
Contributor Author

You also need to remove the sequential marker from setup.cfg

indeed, my dear squid! Just did that 👍

@valeriupredoi
Copy link
Contributor Author

valeriupredoi commented Nov 23, 2022

I don't want to be a conspiracy theorist but I have noticed this ESMF-related test fail on Tool just a few hours ago too - @zklaus any idea where it may come from?

@bouweandela would you be able to give this one last looksee and approve, please, bud? Absolutely no SegFaults from a ton of test runs, we have somehow cured that 🥳

Copy link
Member

@bouweandela bouweandela left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @valeriupredoi 🥇

@bouweandela
Copy link
Member

I don't want to be a conspiracy theorist but I have noticed this ESMF-related test fail on Tool just a few hours ago too - @zklaus any idea where it may come from?

That looks like it could be this issue: numpy/numpy#22623

@valeriupredoi
Copy link
Contributor Author

Brilliant, cheers muchly for the review @bouweandela 🍺 @ESMValGroup/technical-lead-development-team could one kind tech soul merge this please 🍻

@valeriupredoi
Copy link
Contributor Author

I don't want to be a conspiracy theorist but I have noticed this ESMF-related test fail on Tool just a few hours ago too - @zklaus any idea where it may come from?

That looks like it could be this issue: numpy/numpy#22623

Hmmm - that does look like ours indeed, great find, man! But why did it happen only in a very restricted time frame and we don't see it anymore (am on the bus to work on me phone, so a bit hard to check setuptools versions, will do at work)

@valeriupredoi
Copy link
Contributor Author

I don't want to be a conspiracy theorist but I have noticed this ESMF-related test fail on Tool just a few hours ago too - @zklaus any idea where it may come from?

That looks like it could be this issue: numpy/numpy#22623

Hmmm - that does look like ours indeed, great find, man! But why did it happen only in a very restricted time frame and we don't see it anymore (am on the bus to work on me phone, so a bit hard to check setuptools versions, will do at work)

OK I looked at setuptools versions - 65.6 is problematic - and it's the one causing our fails, the other tests that pass all got 65.5 (how the solver decided to jump to 65.6 for just a few Python/OS versions is beyond me, I blame it on magic). Also this comment on the issue you linked, Bouwe numpy/numpy#22623 (comment) is very worrisome - shall we pin setuptools for now?

@bouweandela
Copy link
Member

If I understood that comment correctly, it looks like ESMF should pin setuptools because they are the ones using the numpy build system.

@bouweandela
Copy link
Member

Anyway, let's take this discussion to an issue?

@valeriupredoi
Copy link
Contributor Author

yes and yes 😁

@schlunma schlunma merged commit 680aad1 into main Nov 25, 2022
@schlunma schlunma deleted the no_more_sequential_tests branch November 25, 2022 07:39
@schlunma
Copy link
Contributor

Thanks both 🚀

@valeriupredoi
Copy link
Contributor Author

cheers Manu 🍺

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 this pull request may close these issues.

4 participants