search

openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
712 stars 38 forks source link

[REVIEW]: Caliban: Docker-based job manager for reproducible workflows #2403

Closed whedon closed 4 years ago

whedon commented 4 years ago

Submitting author: @sritchie (Sam Ritchie) Repository: https://github.com/google/caliban Version: 0.4.1 Editor: @diehlpk Reviewer: @lukasheinrich, @arokem Archive: 10.5281/zenodo.4026612

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/c33c8b464103b2fb3b641878722bf8f3"><img src="https://joss.theoj.org/papers/c33c8b464103b2fb3b641878722bf8f3/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/c33c8b464103b2fb3b641878722bf8f3/status.svg)](https://joss.theoj.org/papers/c33c8b464103b2fb3b641878722bf8f3)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@lukasheinrich & @arokem, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @diehlpk know.

✨ Please try and complete your review in the next six weeks ✨

Review checklist for @lukasheinrich

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @arokem

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 4 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @lukasheinrich, @arokem it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf
whedon commented 4 years ago 
Reference check summary:

OK DOIs

- 10.1145/2889160.2891057 is OK

MISSING DOIs

- https://doi.org/10.1109/cvpr.2009.5206848 may be missing for title: Imagenet: A large-scale hierarchical image database

INVALID DOIs

- None
whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

diehlpk commented 4 years ago

@sritchie Could you please check the missing DOI reported above?

sritchie commented 4 years ago

@whedon check references

whedon commented 4 years ago 
Reference check summary:

OK DOIs

- 10.1145/2889160.2891057 is OK
- 10.1109/cvpr.2009.5206848 is OK

MISSING DOIs

- None

INVALID DOIs

- None
sritchie commented 4 years ago

@diehlpk looks like we're good to go!

lukasheinrich commented 4 years ago

hi @sritchie -- part of the described functionality requires Google Cloud Access. Will you provide credits or similar to allow us to test that functionality (I don't have any free credits left)?

sritchie commented 4 years ago

@lukasheinrich and @arokem , of course - I have a Cloud project I can add you both to, and can send you a service account key that you can install on your machine to submit jobs and push containers. Do you want to send me your email address at sritchie09@gmail.com? I'll go ahead and add you right away.

sritchie commented 4 years ago

@lukasheinrich , if you go ahead and send me an email I can get you set up. @arokem got in touch, so I have a little guide written up for you both.

Cheers! Sam

sritchie commented 4 years ago

Hey @arokem, @lukasheinrich and @diehlpk - I responded a week ago with details here, but haven't heard anything back. Let me know if I can assist with anything!

diehlpk commented 4 years ago

Hey @sritchie I think you are fine and the reviewers will proceed with their review. Normally, the review time is about 6 weeks, since we asked the reviewers to finish their review within 6 weeks.

diehlpk commented 4 years ago

Hey @arokem and @lukasheinrich how is your review going?

arokem commented 4 years ago

@diehlpk : I am making slow progress through the checklist. My main remaining items are about testing the various functionality of the library. So far, it's been an enjoyable experience 😄

Regarding community guidelines, I've posted an issue in the software repo.

I also have a small comment for this item:

Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?

The authors tend to make liberal use of superlatives (the word "tremendous" appears more than once). I think that it would be good to use these sparingly.

arokem commented 4 years ago

One more comment, regarding the state of the art. There are two strands of research not currently cited, that I believe could be connected to this software and would better embed the software in context:

The first regards automated docker deployment for scientific software. I am thinking primarily of repo2docker, which implements functionality similar to caliban notebook and binder. This paper could be cited: https://openreview.net/forum?id=BJlR6KTE3X

The second regards automated deployment of scientific computing to the cloud. One set of work there is pywren and related papers (e.g., https://shivaram.org/publications/pywren-socc17.pdf). The other is our own work on cloudknot. If you feel so compelled, you could cite our scipy paper on that: http://conference.scipy.org/proceedings/scipy2018/adam_richie-halford.html.

arokem commented 4 years ago

Relatedly, but definitely not required for the review, some food for thought here: https://github.com/google/caliban/issues/49

arokem commented 4 years ago

I am trying to run caliban cloud and ran into https://github.com/google/caliban/issues/51.

Setting the environment variable seems to resolve that, but I then run into: 

$caliban cloud --nogpu mnist.py
I0723 08:10:11.261835 4394732864 core.py:497] Generating Docker image with parameters:
I0723 08:10:11.262293 4394732864 core.py:498] {'adc_path': None,
 'build_path': '/Users/arokem/tmp/calibam/demo',
 'caliban_config': {},
 'conda_env_path': None,
 'credentials_path': None,
 'extra_dirs': None,
 'job_mode': <JobMode.CPU: 'CPU'>,
 'no_cache': False,
 'package': Package(executable=['python'], package_path='.', script_path='mnist.py', main_module=None),
 'requirements_path': 'requirements.txt',
 'setup_extras': None}
I0723 08:10:11.262627 4394732864 docker.py:614] Running command: docker build --rm -f- /Users/arokem/tmp/calibam/demo
Sending build context to Docker daemon  9.775kB
Step 1/11 : FROM gcr.io/blueshift-playground/blueshift:cpu
 ---> fafdb20241ad
Step 2/11 : RUN [ $(getent group 20) ] || groupadd --gid 20 20
 ---> Using cache
 ---> 0fb794a9d15b
Step 3/11 : RUN useradd --no-log-init --no-create-home -u 501 -g 20 --shell /bin/bash arokem
 ---> Using cache
 ---> 18bce73381b9
Step 4/11 : RUN mkdir -m 777 /usr/app /.creds /home/arokem
 ---> Using cache
 ---> 18c96e44e68c
Step 5/11 : ENV HOME=/home/arokem
 ---> Using cache
 ---> 7a4bb135f05e
Step 6/11 : WORKDIR /usr/app
 ---> Using cache
 ---> 266bb6485852
Step 7/11 : USER 501:20
 ---> Using cache
 ---> e3954e929d29
Step 8/11 : COPY --chown=501:20 requirements.txt /usr/app
 ---> Using cache
 ---> 08d9467b6ea9
Step 9/11 : RUN /bin/bash -c "pip install --no-cache-dir -r requirements.txt"
 ---> Using cache
 ---> d9eaf5a76590
Step 10/11 : COPY --chown=501:20 . /usr/app/.
 ---> Using cache
 ---> fdd5a32ee33f
Step 11/11 : ENTRYPOINT ["python", "mnist.py"]
 ---> Using cache
 ---> bbbce372b9f5
Successfully built bbbce372b9f5
The push refers to repository [gcr.io/landscape-238422/bbbce372b9f5]
45fcaa2b93ad: Preparing 
3c1176c86461: Preparing 
50e8c557ccbf: Preparing 
bd07837419cb: Preparing 
42d270e25e53: Preparing 
c83c21629dcf: Waiting 
a8c372c103ab: Waiting 
4e84efa3db52: Waiting 
25f9bffed627: Waiting 
d4b51844f98b: Waiting 
ed1d1d4a83ac: Waiting 
df7bab7a7925: Waiting 
28ba7458d04b: Waiting 
838a37a24627: Waiting 
a6ebef4a95c3: Waiting 
b7f7d2967507: Waiting 
denied: Token exchange failed for project 'landscape-238422'. Caller does not have permission 'storage.buckets.create'. To configure permissions, follow instructions at: https://cloud.google.com/container-registry/docs/access-control
Traceback (most recent call last):
  File "/Users/arokem/.virtualenvs/caliban/bin/caliban", line 8, in <module>
    sys.exit(main())
  File "/Users/arokem/.virtualenvs/caliban/lib/python3.7/site-packages/caliban/main.py", line 155, in main
    app.run(run_app, flags_parser=cli.parse_flags)
  File "/Users/arokem/.virtualenvs/caliban/lib/python3.7/site-packages/absl/app.py", line 299, in run
    _run_main(main, args)
  File "/Users/arokem/.virtualenvs/caliban/lib/python3.7/site-packages/absl/app.py", line 250, in _run_main
    sys.exit(main(argv))
  File "/Users/arokem/.virtualenvs/caliban/lib/python3.7/site-packages/caliban/main.py", line 145, in run_app
    xgroup=xgroup,
  File "/Users/arokem/.virtualenvs/caliban/lib/python3.7/site-packages/caliban/cloud/core.py", line 637, in submit_ml_job
    image_tag = generate_image_tag(project_id, docker_args, dry_run=dry_run)
  File "/Users/arokem/.virtualenvs/caliban/lib/python3.7/site-packages/caliban/cloud/core.py", line 505, in generate_image_tag
    image_tag = d.push_uuid_tag(project_id, image_id)
  File "/Users/arokem/.virtualenvs/caliban/lib/python3.7/site-packages/caliban/docker.py", line 655, in push_uuid_tag
    subprocess.run(["docker", "push", image_tag], check=True)
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/subprocess.py", line 468, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['docker', 'push', 'gcr.io/landscape-238422/bbbce372b9f5:latest']' returned non-zero exit status 1.

Do the permissions on the project need to be adjusted somehow?

diehlpk commented 4 years ago

@sritchie Can you please add the country (no abbreviation) to the affiliation?

sritchie commented 4 years ago

@arokem , yes, sorry about that... this was an older project, and I hadn't quite synced up permissions yet for the service account. I thought I'd done this for you, but my mistake! This would not come up on your own project.

Give it a try now!

sritchie commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

sritchie commented 4 years ago

@diehlpk whoops, sorry, I just realized I did exactly what you asked me not to do and added "USA", copying the other papers I'd seen. What would you like me to add? "United States", or "United States of America"? Let me know and I'll be happy to make the change.

diehlpk commented 4 years ago

@sritchie I think we like to have United States of America.

sritchie commented 4 years ago

@diehlpk okay, done. let me regenerate

sritchie commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

diehlpk commented 4 years ago

Hi @lukasheinrich how is your review going?

lukasheinrich commented 4 years ago

Hi @diehlpk I will be able to finish the review next week. Apologies for the delay.

diehlpk commented 4 years ago

Hi @sritchie have you addressed the remarks of @arokem in the paper?

sritchie commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

sritchie commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

sritchie commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

sritchie commented 4 years ago

@arokem and @diehlpk , this should cover the review by @arokem . Thanks to you both, and please let me know what else I can address.

diehlpk commented 4 years ago

Hey @arokem and @lukasheinrich how is your review going?

arokem commented 4 years ago

I just checked the remaining boxes. From my point of view, this paper is ready to be accepted.

diehlpk commented 4 years ago

Hey @lukasheinrich when do you anticipate to finish the review?

diehlpk commented 4 years ago

Hey @lukasheinrich are you still available for the review? If you can not finish the review, it would be great if you let me know, so I could start to find some replacement for you.

diehlpk commented 4 years ago

@sritchie @lukasheinrich told me to finish the review by next week.

sritchie commented 4 years ago

Thanks for the update, @diehlpk !

lukasheinrich commented 4 years ago

@sritchie I successfully tested caliban locally but still have trouble accessing the cloud. Are the credentials you sent me still valid?

ERROR: (gcloud.container.images.list-tags) Access denied: gcr.io/landscape-238422/09839a2db96e

sritchie commented 4 years ago

Hey @lukasheinrich , I realize I had not added your gmail account to the project, which is probably causing the issue here. Mind giving it another run? If you run into trouble I'll debug deeply and make sure you're all set, but I think this should get you going.

Cheers!

lukasheinrich commented 4 years ago

Hi @sritchie,

thanks! This seems to work and I'm able to push some new images.

Cheers!

lukasheinrich commented 4 years ago

Hi @sritchie

I successfully ran through the examples and went through the checklist and am generally satisfied (all boxes are checked)

Generally I think it would be useful if the demo tutorial was included in the docs (and not link back out to github) and agree with @arokem that it would be useful to cite binder/repo2docker.

Apologies again for the delay. This seems like a very useful piece of software.

sritchie commented 4 years ago

Thanks @lukasheinrich! I did mention boot2docker and binder in related work, so thanks for that:

image

Yes, I think you're probably right about mirroring the tutorial. I bet I can do some readthedocs magic and get the docs site to import the README wholesale...

diehlpk commented 4 years ago

I successfully ran through the examples and went through the checklist and am generally satisfied (all boxes are checked)

@lukasheinrich It seems that

Functionality: Have the functional claims of the software been confirmed?

is not checked. Could you please verify?

diehlpk commented 4 years ago

@lukasheinrich @arokem Thanks for finishing the review.

diehlpk commented 4 years ago

Yes, I think you're probably right about mirroring the tutorial. I bet I can do some readthedocs magic and get the docs site to import the README wholesale...

@sritchie Let me know once you accomplished that and after that you can work on following items:

I can then move forward with accepting the submission.