nus-apr / auto-code-rover

A project structure aware autonomous software engineer aiming for autonomous program improvement. Resolved 30.67% tasks (pass@1) in SWE-bench lite and 38.40% tasks (pass@1) in SWE-bench verified with each task costs less than $0.7.
Other
2.67k stars 276 forks source link

Change build so that this will run in Docker on arm64/aarch64 #22

Closed jimwhite closed 5 months ago

jimwhite commented 5 months ago

Changes so folks using on arm64, like Apple silicon Macs, can run natively in Docker.

Use the official Miniconda DockerHub image (continuumio/miniconda3). The source for that Dockerfile is here: https://github.com/ContinuumIO/docker-images/tree/main/miniconda3).

Remove the hashes from the environment.yaml because they aren't the same between amd64 and arm64.

Tweakage to the README.

One problem with running on arm64 is that Miniconda's repo doesn't have Python 3.6. There are two things to do about that. One is to change the task runner to skip tasks that fail configuration (instead of crashing) and the other is to see if there is a reasonable way to get Python 3.6 working in this setup.

zhiyufan commented 5 months ago

Hi @jimwhite , thanks for contributing. We are currently testing the docker in Apple silicon macs, will merge if it works!

crhf commented 5 months ago

Hi @jimwhite, you have removed this dependency from dockerfile:

RUN echo ttf-mscorefonts-installer msttcorefonts/accepted-mscorefonts-eula select true | debconf-set-selections
RUN apt install -y ttf-mscorefonts-installer

This is dependency for several tasks. Did you remove it by mistake?

jimwhite commented 5 months ago

Hi @jimwhite, you have removed this dependency from dockerfile:

RUN echo ttf-mscorefonts-installer msttcorefonts/accepted-mscorefonts-eula select true | debconf-set-selections
RUN apt install -y ttf-mscorefonts-installer

This is dependency for several tasks. Did you remove it by mistake?

I omitted it on purpose and included instructions in the comments on how to add it manually if it is desired. That font package is not required or really even used by ACR or SWE-beanch afaict. It seems to be a personal developer preference for use during development.

Including that package has two significant problems:

1) It isn't in the Debian 'main' distribution channel, it requires the 'contrib' channel. The official Miniconda image is built on Debian bullseye-slim which does not include the 'contib' channel. Adding the 'contrib' channel to this Dockerfile would increase its complexity too much for something that isn't actually needed.

2) Installing the msttcorefonts package requires accepting the EULA (End User License Agreement), as you can see by the "echo" message which is apparently intended to prompt the user to run that command manually. That seems like a pretty inappropriate assumption to me, especially for software distributed under the GPL, but something that the user should be making a conscious decision about.

This relates to a less critical issue which is why there are so many packages being installed which are not used by ACR or SWE-bench. It seems pretty clear those are idiosyncratic choices made by a developer for their personal dev environment. Perfectly understandable as a research project but as this software gets used by a broader community there should be more separation of those concerns. A perfectly reasonable thing to do would be to add a separate file (e.g. Dockerfile.developer) that included a reasonable development-oriented configuration. I think this configuration should only include what is required for running ACR and (it's fork of) SWE-bench. I didn't try to omit the other unnecessary packages because that is beyond the scope of this change (i.e. having a Dockerfile that will run on the popular architectures, not just AMD64).

yuntongzhang commented 5 months ago

The font package was added because some target projects in SWE-bench listed those as system-level dependencies. Most other packages in the Dockerfile were also added due to the same reason. Sometimes these system-level dependencies are required to set up projects in SWE-bench, or to run testcases in those projects.

I agree that these should not be handled in the ACR Dockerfile. Perhaps in the future we can have a Dockerfile for ACR only (for running on new issues), and a Dockerfile.swebench for running SWE-bench tasks. Anyway that's out of scope for this PR.