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

Feature/auto-push-docker-images and version upgrades #342

Merged
merged 35 commits into from
Dec 13, 2021

Conversation

KaleabTessera
Copy link
Contributor

What?

  • Added a github action to auto-push docker images
  • Updates to package version to fix ModuleNotFoundError: No module named 'chex' .

Why?

How?

  • Updated github actions file.
  • Updated setup.py to change auto install chex and updated package versions.

Extra

@KaleabTessera KaleabTessera marked this pull request as ready for review December 13, 2021 09:47
Copy link
Contributor

@DriesSmit DriesSmit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks @KaleabTessera 🙂

Copy link
Contributor

@mmorris44 mmorris44 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some small notes. Otherwise looks great!

@@ -6,6 +6,7 @@ on:
pull_request:
branches: [develop]

# TODO(Kale-ab) Specify mava container to run tests on.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO still un-done. Is this for a future PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Once we have an instadeepct dockerhub account, I will pull a container from there in the workflow. Currently, we are using my personal dockerhub so I haven't added it here.

@@ -4,6 +4,7 @@ on:
pull_request:
branches: [main]

# TODO(Kale-ab) Specify mava container to run tests on.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar un-done TODO

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented above.

@@ -1,6 +1,6 @@
##########################################################
# Core Mava image
FROM nvcr.io/nvidia/cuda:11.4.2-cudnn8-runtime-ubuntu20.04 as mava-core
FROM registry.kao.instadeep.io/library/nvidia/cuda:11.4.2-cudnn8-runtime-ubuntu20.04 as mava-core
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this standard practice for us going forward? To use a InstaDeep-hosted image rather than an nividia one?

Makefile Show resolved Hide resolved
@KaleabTessera KaleabTessera merged commit 0c87b05 into develop Dec 13, 2021
@KaleabTessera KaleabTessera deleted the feature/docker-ci branch December 13, 2021 14:28
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.

3 participants