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

Combine base notebook cpu gpu images #151

Closed
wants to merge 1 commit into from

Conversation

JessicaBarh
Copy link
Contributor

This Relates to PR #129 that is being split up into smaller PRs for better review.

Resolves part of: #146

Refactored base-notebook-gpu. upstream-equivalent-notebook-gpu will be built on the fly by this CI from a SHA-pinned pull of GPU-Jupyter to create the GPU equivalent to docker-stacks' datascience-notebook, which is our CPU upstream image. This lets both CPU and GPU versions of base-notebook be identical except for the upstream image.

@sylus sylus requested a review from zachomedia November 9, 2020 17:11
@sylus
Copy link
Member

sylus commented Nov 9, 2020

@zachomedia you are going to have to review this as can't quite make sense of it.

@JessicaBarh JessicaBarh changed the title Combine cpu gpu images Combine base notebook cpu gpu images Nov 9, 2020
@JessicaBarh JessicaBarh deleted the combine-cpu-gpu-images branch November 11, 2020 04:57
@JessicaBarh JessicaBarh restored the combine-cpu-gpu-images branch November 11, 2020 05:08
@JessicaBarh JessicaBarh reopened this Nov 11, 2020
.github/workflows/build-cpu.yml Show resolved Hide resolved
.github/workflows/build-cpu.yml Outdated Show resolved Hide resolved
base-notebook/Dockerfile Outdated Show resolved Hide resolved
base-notebook/Dockerfile Outdated Show resolved Hide resolved
base-notebook/Dockerfile Outdated Show resolved Hide resolved
@ca-scribner
Copy link
Contributor

closes #122

@ca-scribner ca-scribner linked an issue Nov 17, 2020 that may be closed by this pull request
@JessicaBarh JessicaBarh marked this pull request as ready for review November 17, 2020 15:59
@ca-scribner
Copy link
Contributor

@zachomedia this is ready for you to review. The main change is that instead of:

  • dev running gpu-jupyter to create a Dockerfile
  • dev copies above Dockerfile into the top of base-notebook-gpu
    this update does:
  • CI runs gpu-jupyter to make a Dockerfile that is a gpu-enabled equivalent to the docker-stacks file we inherrit
  • base-notebook for cpu and gpu can be the same (because all gpu-specific stuff is in the preceeding image) and now takes an ARG for its base container (used by CI to build either CPU or GPU flavor

That, and there's now CI enabled for PR's to master in addition to merging into master, which allows debugging CI without actually merging into master.

@sylus
Copy link
Member

sylus commented Nov 25, 2020

@zachomedia can you review this today?

@sylus sylus closed this Nov 27, 2020
@JessicaBarh JessicaBarh deleted the combine-cpu-gpu-images branch December 18, 2020 19:01
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.

Tensorflow missing cuDNN drivers
3 participants