Update Dockerfile Julia 1.3.1->1.5.3

[postylem]

Is there any reason not to update to Julia 1.5.3?

[bzinberg]

Looks good on reading. To minimally test it, can you build and run the image and verify that the notebooks still run?

[bzinberg]

why /usr/local/bin rather than /usr/bin? (answer might be "the convention is pretty vague anyway," just wondering)

[postylem]

just because @fplk suggested it :) do you have a preference?

[fplk]

/usr/bin is for distribution-managed binaries.

[postylem]

I tested with Dockerfile.ubuntu-2004, and it works (I can build the docker image and run the notebooks) TF now supports python 3.8, so I updated to just install tensorflow and not use the tf-nightly wheel, in this last commit.

I tested with Dockerfile and it doesn't build (something the matter with virtualenvs and python 2 vs 3). But I have this same issue in the current probcomp:master branch too, so I don't think it is related. (this issue is mentioned in issue 61)

[fplk]

Nested Virtual Environments Usually Antipattern
I know of very few scenarios in which using virtual environments inside containers is a good idea, since essentially you are just nesting virtual environments without benefit at this point - if you are already inside a container that only adds complexity.

Upgrading to 20.04 Image?
Also, Ubuntu 16.04 either is in ESM or will be very soon - maybe it makes sense to fully upgrade to the 20.04 image at this point? @marcoct @bzinberg

Comments Regarding Upgrades in Near Future
Let me also repeat two remarks regarding upgrading Julia.

Xuan mentioned:

I think Julia 1.5 should be fine. There might be issues with Genify.jl when Julia 1.6 comes along (or so McCoy tells me), but I haven't encountered them myself, and Genify is not used by other parts of the Gen ecosytem (yet).

In addition, Marco commented:

[W]e are good to upgrade. I added Julia 1.5 to the Travis build in the hackathon and it passes. (edited)
Depending on what version of Gen gets installed by the Docker file, it may not have been tested with Julia 1.5. I don't anticipate there were any issues with it, but it would probably also be good to update the Docker file to run with the latest version of Gen. That seems more involved though, since the notebooks would need to be tested against the latest version of Gen -- there is some non-negligible probability that some of them might have become broken when run in with the latest version of Gen -- and testing this is not currently automated. So, to summarize, updating Julia should be fine; updating Gen might be more involved. Both seem important, and if anything updating Gen version is more important.

[fplk]

Consolidated Base Image with Gen Main Project
One important thing I just remembered: I actually wanted to use the main Gen image for this, see probcomp/Gen.jl#312

The original idea was that if we have solid Gen images for CPU and GPU projects like gen-quickstart and GenSceneGraphs could inherit from it. You will see that the Julia installation there is virtually identical to what I proposed above (it's just still on 1.5.1). When I find a free minute I can finish that PR and just adapt this repo to build upon that image. That's probably the cleanest solution. Afterwards, I can chat with Ben how and whether to base GSG on the same base image. Makes sense?

[bzinberg]

Missed the replies until now, sorry for the delay.

I recommend we keep the scope of this PR small so that @postylem can unblock his downstream work, and move most of the above discussion to a separate issue where it can be discussed on a separate timeline.

TBH, personally I think we should have this PR literally just change all 1.3s to 1.5s -- that makes it easier to review at the level of "this at least doesn't introduce any new bugs," unblocks @postylem's work, and substantive changes can be drafted and tested in a follow-up PR which @postylem may or may not want to drive.

Thoughts?

[postylem]

As is, all this PR does is change the 1.3 to 1.5, but in a parametrized way... I don't think that should introduce any bugs, could it? I could change that back the link to /usr/bin rather than /usr/local/bin, so that's really all it does, or I could do as you say and just go back to literally changing the number, I will defer to whatever you think Ben. For what I am using it for (the gen pset), I am currently just using the 20.04 version of the dockerfile with v1.5, and it is working well.

[bzinberg]

I don't think that should introduce any bugs, could it?

Sadly, bugs are always possible, especially when it looks like they're impossible. But I agree, it's reasonable to keep the parameterized version number with the known risk (believed unlikely) of having to later go back and fix some unseen issue.

Given that you've verified:

• The 20.04 image as changed in this PR builds and works
• The 16.04 image is broken both before this PR and after this PR

I'm ok with merging the PR in its current state. @fplk, agree?

[fplk]

Merging seems fine. However, one nitpick:

RUN             python3 -m pip install --upgrade pip
RUN             python3 -m pip install jupyter jupytext matplotlib tensorflow torch

I think this creates two AUFS layers. Why? Seems like it would be better to just do:

RUN python3 -m pip install --upgrade pip && python3 -m pip install jupyter jupytext matplotlib tensorflow torch

Similarly: Is this really necessary?

RUN             git config --global user.name "Gen User"
RUN             git config --global user.email "[email protected]"

Or can we do this?

RUN git config --global user.name "Gen User" && git config --global user.email "[email protected]"

And finally, it seems like the original Dockerfile still exists. Is this intended or should we go the step and replace the old Dockerfile with 20.04?

[fplk]

@bzinberg The parametrization code was provided by me and is exactly what we use for the MCS image, so I'd consider it stable enough. If it should break unexpectedly, we both would like to know and would fix it with high priority.

[fplk]

Btw.: @bzinberg I think you need to merge this whenever you feel it's ready - I don't have rights to do so and I assume the same is true for @postylem

@postylem You can decide whether you want to address my nitpicks or not - don't think they are showstoppers, just wanted to mention it.

[postylem]

I did chain those commands with &&, as you suggest @fplk (it makes sense to me, and I'd I wondered about that, though I'm rather unclear about when/whether fewer layers is desirable in general), but I left the original Dockerfile there. I thought that change should be a seperate PR, if the point of this one was just to change the julia version, not also the ubuntu version. Though, given that the ubuntu 16.04 Dockerfile seems to be broken, I could start a PR for that change too (or include it in this one, if @bzinberg thinks that's a good idea).

[bzinberg]

Migrating things punted by this PR to issues:

• Broken Ubuntu 16.04-based image (Dockerfile): tracked already in Syntax Error Building Docker #61

I think that might be all?

[bzinberg]

Just built and pushed the new image as probcomp/gen-quickstart:2021-03-23.001.