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

Add GPUCurrentLoad dashboard plots #2944

Merged
merged 1 commit into from
Aug 12, 2019
Merged

Conversation

mrocklin
Copy link
Member

@mrocklin mrocklin commented Aug 9, 2019

A first step towards fixing rapidsai/dask-cuda#36

@pentschev
Copy link
Member

A bit of an unrelated question, but I thought you wanted to keep GPU-specific code mostly out of dask/distributed. This is just for my own understanding, what's the current policy on whether GPU code should go in here rather than dask-cuda?

@mrocklin
Copy link
Member Author

mrocklin commented Aug 9, 2019

It's a good question. I could imagine moving this around later in the future.

There was some non-trivial amount of work to make the dashboarding general enough to take information like this. I was also able to find a way to keep things nicely separate. I also have decent confidence that this code won't need much experimentation in the future, and will be low churn.

@mrocklin
Copy link
Member Author

mrocklin commented Aug 9, 2019

For example, we defined cuda_serialize in distributed because the code changes needed for it are mostly the same as the code changes needed for dask_serialize. The infrastructure is there. Things like the dask-cuda-worker code is changing rapidly enough that I'd prefer to keep it external for a while. I could imagine moving it in once it's more stable.

@pentschev
Copy link
Member

Ok, makes sense. Thanks for the clarifications @mrocklin .

Now going back to this PR, would you like me to test it out and review it?

@mrocklin
Copy link
Member Author

mrocklin commented Aug 9, 2019

Review would be welcome

@pentschev
Copy link
Member

It looks good to me, I added a couple of minor comments. I don't have experience with bokeh, superficially it looks fine for me, but maybe someone else with bokeh experience could take a look at that, otherwise, good to be merged by me.

@pentschev
Copy link
Member

LGTM, looking forward to see this being used by more people!

@mrocklin mrocklin merged commit b27f7b7 into dask:master Aug 12, 2019
@mrocklin mrocklin deleted the gpu-dashbaord branch August 12, 2019 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants