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

Begin to support Julia #38

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Conversation

eschnett
Copy link
Contributor

After discussing with @sritchie yesterday I looked into supporting Julia as language. I found the following:

It is straightforward to install Julia into the directory that is packed up by Docker. Since a Julia install has many files, this is too slow.

I instead opted to install Julia in a similar way as Python. There is no good Ubuntu package for Julia; the available ones are very outdated. Instead, Julia needs to be installed by downloading a binary. Julia has a built-in package manager, and a simple Julia command will then install all package dependencies. In the long run, it might be nice to base the Docker template onto a container that already has Julia installed, but that doesn't seem necessary at the moment where I'm just experimenting with Julia.

The enclosed "pull request" should make it easy to see what I changed. I decided to pass a parameter julia_url in the same was as e.g. requirements_path and handle it in a similar way. julia_url is the URL where an architecture-specific binary of Julia is available. This seems to work nicely.

My question is: What is a good way for the user to define julia_url and pass it to _dependency_entries? I'm still in a phase where I need to find my way around the source code and configuration files.

I would, of course, also be happy about other comments about my approach.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@eschnett
Copy link
Contributor Author

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@sritchie
Copy link
Collaborator

@eschnett , thank you for this! I should have some time tonight to take a look and give feedback on how we can get this integrated.

@sritchie sritchie self-requested a review July 15, 2020 20:11
@codecov
Copy link

codecov bot commented Jul 15, 2020

Codecov Report

Merging #38 into master will increase coverage by 1.14%.
The diff coverage is 13.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #38      +/-   ##
==========================================
+ Coverage   54.31%   55.46%   +1.14%     
==========================================
  Files          31       31              
  Lines        3183     3195      +12     
==========================================
+ Hits         1729     1772      +43     
+ Misses       1454     1423      -31     
Impacted Files Coverage Δ
caliban/docker/build.py 31.44% <13.33%> (-1.27%) ⬇️
caliban/util/metrics.py 100.00% <0.00%> (ø)
caliban/config/__init__.py 100.00% <0.00%> (ø)
caliban/history/cli.py 13.40% <0.00%> (+0.06%) ⬆️
caliban/resources/caliban_launcher.py 100.00% <0.00%> (+66.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e175c2...c5eb7e9. Read the comment docs.

@eschnett
Copy link
Contributor Author

I'm also working on a Slurm backend, i.e. connecting to a Slurm scheduler on a remote system via ssh. To simplify things for me while I develop, this is now living on the same branch. Since it affects different files (platform/slurm) I hope it doesn't make it more difficult to see what I changed. I'll follow your submission guidelines when/if this is ready to be included.

@eschnett eschnett marked this pull request as ready for review July 17, 2020 15:11
@sritchie
Copy link
Collaborator

hey @eschnett , I was thinking about Julia support - I think I can take your example and build you a docker base image with Julia in it, and maybe add a "julia" flag to the calibanconfig to privilege this container in the way that the recent #39 gives some special syntax for these deep learning VMs.

I THINK that if you don't have requirements.txt or setup.py, we won't do any python-specific things when you run caliban commands.

I don't know Julia yet, but based on https://julialang.github.io/Pkg.jl/v1/toml-files/

It looks like the presence of a Project.toml (or Manifest.toml?) is what we need to look for, and trigger julia --eval 'using Pkg; Pkg.instantiate()'" if it exists. Is that right? Would that be enough (along with the julia installation) to get someone a working arena?

Then, the current code should work if you use caliban run shell_script.sh, but I can update the code here to detect a .jl file and issue the appropriate equivalent to python -m.

@eschnett
Copy link
Contributor Author

@sritchie This looks approximately correct. A few details:

  • There are several versions of Julia available. It's a younger language, so people might care about which version they use (e.g. 1.4, or 1.5 once it's released). There should be several base images to choose from.
  • Yes, the Project.toml is the relevant file
  • Julia packages are by default installed into a directory .julia in the home directory. That won't work since this isn't picked up by Docker. We need to set a few environment variables to put the packages into a user-writable directory in the Docker image. I chose /tmp because I couldn't find another writable one. Maybe we could prepare a writable /var/julia or so?

@eschnett
Copy link
Contributor Author

Regarding python -m: It's less common to have sub-packages in Julia than having modules in Python. It would make sense to accept the name of a function, possibly qualified with a package name, and look for that function in the current package. The respective Julia code would be similar to (untested)

julia -e 'using {package}; {function}()'

@sritchie
Copy link
Collaborator

@eschnett , I've just released 0.3.0 at https://github.com/google/caliban/releases/tag/0.3.0 with some of the work you've been doing. Thank you! I put a julia_version key into calibanconfig.json that we don't yet use; I'm going to take a look now and see if we can get this in ASAP so folks can start using Julia.

I take it you've been developing with this feature. Anything missing for Julia support that you've noticed, or is this it?

Thank you again!

@eschnett
Copy link
Contributor Author

eschnett commented Jul 24, 2020 via email

@sritchie
Copy link
Collaborator

Interesting; I wonder, is it actually a GOOD thing if the Julia installation shares a conda env with the python scripts? Then someone could, for example, use a project that has a bunch of their own custom scripts, and call those (which require dependencies) from Julia.

There may be some subtlety here that I'm not reading correctly, of course.

Another question. If the conda environment is activated in the base container, is "system Python", from Julia's perspective, actually the Conda env that we've already set up?

@eschnett
Copy link
Contributor Author

eschnett commented Jul 24, 2020 via email

@eschnett
Copy link
Contributor Author

eschnett commented Jul 30, 2020 via email

@eschnett
Copy link
Contributor Author

eschnett commented Aug 7, 2020

@sritchie How should we continue to work on this pull request? It's in a good state, and I've been using this for my Julia development for a while. Is there something missing or something that should be cleaned up?

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