-
Notifications
You must be signed in to change notification settings - Fork 224
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
WIP: Fix random failures in Windows CI #758
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
fd7a16c
to
0e4a6bf
Compare
/test-gmt-dev try 5 |
Windows tests still failing with I've been reading up the threads at astropy/astropy#7404 and joblib/joblib#806 and think it might related to how we are reading in the grids using |
I think the tests fail randomly with GMT 6.1.1, but always fail with GMT dev, and the error messages are also different. |
Ok, then best to test on GMT dev (6.2.0) which consistently fails (rather than waiting for random failures), the PermissionError I got above is from the GMT Latest/Dev tests actually. I'll need try to resolve #829 locally on my end to be able to debug this 😞 |
Keep it outside the `with GMTTempFile()` block
Ok, so the tests on GMT Dev 6.2.0 started failing on 13 January 2021 (see https://github.com/GenericMappingTools/pygmt/runs/1691984838?check_suite_focus=true#step:12:437), the previous one on 12 January 2021 was fine (https://github.com/GenericMappingTools/pygmt/runs/1685025652?check_suite_focus=true#step:12:727): The commits on GMT around that date (see https://github.com/GenericMappingTools/gmt/commits/master?after=1456fa60dd7ff0722e53b1e93e983c35b54b10a4+34&branch=master) are as follows: The Python dependencies (e.g. |
We're using the GMT dev build from conda-forge's dev channel. It seems at that date we bumped |
Ah ok, so from PR conda-forge/gmt-feedstock#129 then. There's 117 commits in between 29 Nov 2020 to 12 January 2021 to check at GenericMappingTools/gmt@c94e83f...98bb060 😅. List of candidates: |
Perhaps it's easier to test on the GMT side. The GMT CI is building the GMT codes on Windows, so what we need to do is:
Edit: Trying in GenericMappingTools/gmt#4745 |
Not quite sure I get you. Do you mean compiling each of 117 GMT dev versions and testing it with PyGMT? That seems like a lot of computing power needed... |
We can use binary search to reduce the number of builds. Ideally, we can find the bug in less than 7 builds (2**7=128 > 117, the math may be wrong). |
Very smart! Let us know when you find it. |
Based on the tests in GenericMappingTools/gmt#4745, GenericMappingTools/gmt@b16cc28 (i.e., GenericMappingTools/gmt#4581) is the commit that introduced the bug. |
Just following up, issue appears to be GMT not closing a file properly, should be fixed in GenericMappingTools/gmt#4777. What we should do here in PyGMT is to remove the xfails for GMT > 6.2.0 (i.e. for the dev tests). The GMT conda-forge dev version merged at conda-forge/gmt-feedstock#134 (and conda-forge/gmt-feedstock#135) should include this fix so we can test it in our CI. |
This PR tries to fix the random failures that sometimes still pass. These tests are not marked as |
Right, got a bit confused as we were trying so many things to see what was broken. So can we close this PR then? |
Description of proposed changes
All PyGMT tests, excluding a few marked as
xfail
orxskip
, should pass on all platforms (Linux/macOS/Windows).However, the tests sometimes fail on Windows. Generally, there are two different types of failures:
OSError: exception: access violation reading 0x000001E880146FBC
.It's difficult to debug, as none of us are developing PyGMT on Windows.
grdcut
and/orgrdfilter
sometimes fail, with the error messages shown below. It turns out that the file exists but has a zero size, so it's an invalid netCDF file. However, it's still puzzling why the file size is zero.Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.Notes
/format
in the first line of a comment to lint the code automatically