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

Critical Testing Instability | Dask Version #2769

Closed
wants to merge 1 commit into from

Conversation

marqh
Copy link
Member

@marqh marqh commented Sep 28, 2017

The first commit on this branch removes white space from the README.md: a no change change

this cascades a chain of CI testing failure ERRORs, of the form:

  File "/home/travis/miniconda/envs/test-environment/lib/python2.7/site-packages/Iris-2.0a0-py2.7-linux-x86_64.egg/iris/cube.py", line 2130, in __getitem__
    cube_data, keys)
  File "/home/travis/miniconda/envs/test-environment/lib/python2.7/site-packages/Iris-2.0a0-py2.7-linux-x86_64.egg/iris/util.py", line 690, in _slice_data_with_keys
    data = data[this_slice]
  File "/home/travis/miniconda/envs/test-environment/lib/python2.7/site-packages/dask/array/core.py", line 1222, in __getitem__
    index2 = normalize_index(index, self.shape)
  File "/home/travis/miniconda/envs/test-environment/lib/python2.7/site-packages/dask/array/slicing.py", line 762, in normalize_index
    raise IndexError("Too many indices for array")
IndexError: Too many indices for array

This does has not appeared on the last run of master testing, which was last run ~28 days prior to this run

This previous run was using dask 0.15.2
https://travis-ci.org/SciTools/iris/jobs/270360216#L1266

This run is using dask 0.15.3
I can reproduce this failure locally using dask 0.15.3

It is not clear to me that this minor version change is the trigger for the failure mode, it is just a potential clue

This is a blocker for incoming Pull Requests, and hence I consider it a significant and time critical issue

@pelson pelson added the Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form label Sep 28, 2017
@pelson pelson added Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form labels Sep 28, 2017
@pp-mo
Copy link
Member

pp-mo commented Oct 2, 2017

It appears this problem is introduced in other changes (i.e. subsequent to the "masked_array" PR) wrapped up in the 0.15.3 release.
However, it also seems to have been fixed in a subsequent commit (i.e. since 0.15.3 was cut).
It's the one-line code change here : https://github.com/dask/dask/pull/2719/files#diff-e37700c40af1928a965532126ff42d1eL754

I haven't yet checked what other effects the newer 0.15.3 release might have on the "dask_mask_array" feature branch.

Of course, if you are intending anything specifically dask-related, you need to be prepared for the ground to shift radically (again!) when we merge "dask_mask_array".
Hopefully, that will be sometime in the next 2-3 weeks.

@marqh
Copy link
Member Author

marqh commented Oct 4, 2017

@pp-mo thank you for the feedback

given that the bug looks like it might be fixed upstream, would you recommend pinning to dask 15.2 temporarily to enable testing to run, then adopting new dask once it is in a release?

@pp-mo
Copy link
Member

pp-mo commented Oct 9, 2017

@marqh thanks for pushing this.
We are going to be tidying up the remaining effort for Iris 2 over the next 2 weeks,
so this will need resolving.

In the meantime, I see that the Dask people have now cut a 0.15.4, so that probably fixes the problem (no need to pin). dask/dask#2751
It's on conda-forge already, anyway : conda-forge/feedstocks@b137940
Can you just give this a poke + just see if it now works ?

Of course, that might also introduce some other problem ..
It does seem they are moving rather quickly just now, though they seem to be mostly localised changes.

Also in the meantime, amusingly it seems you need to submit a CLA (!!)
( @pelson added a test hook a few weeks back to check all PR authors have a CLA in place )

@djkirkham
Copy link
Contributor

@pp-mo @marqh One of the Python 3.5 builds is erroring for an apparently different reason

@djkirkham
Copy link
Contributor

I think it's this issue: cython/cython#1880. I think what's happened is that netCDF4 has been compiled against Python >=3.5.2, which has the symbol _PyThreadState_UncheckedGet but our Travis build that's failing is running with Python 3.5.1, which doesn't define that symbol. The other 3.5 tests are using Python 3.5.4, probably because some package not specified in minimal-conda-requirements.txt is specifying that as a minimum.

@pp-mo
Copy link
Member

pp-mo commented Oct 10, 2017

#2774 recently merged ought to fix this.
@marqh can you please try rebasing this against the latest master ?
That is, make this into a real PR, or post the actual changes you are trying to move forward.

@marqh
Copy link
Member Author

marqh commented Oct 10, 2017

@marqh can you please try rebasing this against the latest master ?
That is, make this into a real PR, or post the actual changes you are trying to move forward.

@pp-mo this is not a real PR, it is a demonstration that a null PR failed testing

if this is no longer the case, I will close this PR (or you can)
looking at
https://github.com/SciTools/iris/pulls
at 12:26Z then no PRs are passing tests, so I'll hold off closing until i see tests passing. If you see this and want to close this PR, you are welcome to

@pp-mo
Copy link
Member

pp-mo commented Oct 10, 2017

this is not a real PR, it is a demonstration that a null PR failed testing

Ok, I thought that must be what is meant.

looking at https://github.com/SciTools/iris/pulls at 12:26Z then no PRs are passing tests, so I'll hold off closing until i see tests passing

In that case I think it's no different from having all tests pass on the master commit d442b4, got from merging #2774
... which we do have, so I'll close this.

@pp-mo pp-mo closed this Oct 10, 2017
@pp-mo pp-mo mentioned this pull request Oct 10, 2017
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants