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.

Sign up for GitHub

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

Jump to bottom

Bugfix/dockerfile no module found #344

Merged
merged 12 commits into from
Jan 6, 2022
Merged

Bugfix/dockerfile no module found #344

merged 12 commits into from
Jan 6, 2022

Conversation

KaleabTessera
Copy link
Contributor

What?

Changed the dockerfile to use virtual envs.

Why?

Recent dockerfiles had issues with finding the mava package.
Error :

Traceback (most recent call last):
  File "examples/petting_zoo/sisl/multiwalker/feedforward/decentralised/run_mad4pg.py", line 26, in <module>
    from mava.systems.tf import mad4pg
ModuleNotFoundError: No module named 'mava'

How?

Updated dockerfile.

Extra

  • Also switched the dockerhub repo that images get pushed to.
  • Added info on how to use pre-built docker images from dockerhub.

@KaleabTessera KaleabTessera self-assigned this Jan 5, 2022
@KaleabTessera KaleabTessera added the bug Something isn't working label Jan 5, 2022
mmorris44
mmorris44 reviewed Jan 5, 2022
View reviewed changes
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.

A few minor concerns. Looks good otherwise!

Dockerfile Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
arnupretorius
arnupretorius approved these changes Jan 6, 2022
View reviewed changes
Copy link
Collaborator

@arnupretorius arnupretorius left a comment

Choose a reason for hiding this comment

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

Thanks @KaleabTessera! :) Looks good. We can look at a export path based solution perhaps sometime in the future. But for now, since this works we keep it.

README.md
```
docker run --gpus all -it --rm -v $(pwd):/home/app/mava -w /home/app/mava instadeepct/mava:tf-core-latest python examples/debugging/simple_spread/feedforward/decentralised/run_maddpg.py --base_dir /home/app/mava/logs/
```
- For windows, replace `$(pwd)` with `$(curdir)`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have we tested this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I have tested the linux version, but not the windows version (as I don't have a windows device). It technically should works since we do something similiar in the makefile (

Mava/Makefile

Line 11 in 8c4cee2

ifeq ($(PWD),)
).

@KaleabTessera KaleabTessera merged commit 0c2a143 into develop Jan 6, 2022
@KaleabTessera KaleabTessera deleted the bugfix/dockerfile-no-module-found branch January 6, 2022 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arnupretorius arnupretorius arnupretorius approved these changes

@mmorris44 mmorris44 mmorris44 approved these changes

@DriesSmit DriesSmit Awaiting requested review from DriesSmit DriesSmit is a code owner

Assignees

@KaleabTessera KaleabTessera

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@KaleabTessera @arnupretorius @mmorris44