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

fixes #419 and remove optimized dask selection #420

Merged
merged 7 commits into from Oct 6, 2017

Conversation

Projects
None yet
2 participants
@nickhand
Member

nickhand commented Oct 5, 2017

This fixes two issues arising in v0.2.7 on NERSC:

  • #419: only slices that do not change the collective size, not local size do not need to be evaluated
  • dask optimized selection is buggy and can fail in a very bad way, at the time of a compute() call, breaking basically the whole code. After some more thought, I think it's best to not fiddle with the dask internals as much as possible, given that the internal API can change at any time. Also, it's a hard piece of code to debug and maintain, so it seems best to remove for both the future usability of the code and future maintenance

Thoughts, @rainwoodman?

@nickhand

This comment has been minimized.

Member

nickhand commented Oct 5, 2017

I've confirmed this fixes my recent hanging issues on NERSC, and we should push a v0.2.8 once @rainwoodman is on board.

@rainwoodman

This comment has been minimized.

Member

rainwoodman commented Oct 6, 2017

I always prefer writing 20% of code to cover the 80% case -- so yes go ahead and remove the dask internal stuff!

@nickhand nickhand merged commit cf81fff into master Oct 6, 2017

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.2%) to 95.313%
Details

@nickhand nickhand deleted the remove-empty-slice-check branch Oct 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment