feat: Add Dockerfile and GitHub Actions based CI #12

matthewfeickert · 2022-01-19T04:19:57Z

Before submitting

Please complete the following checklist when submitting a PR:

All new features must include a test sequence.
If you've fixed a bug or added code that should be tested, add a
test sequence below for it to be checked by other developers.
All new functions and code must be clearly commented and documented.
If you do make documentation changes, make sure that the docs build and
render correctly by running ./doc/makedoc.
Add a new entry to the madanalysis/UpdateNotes.txt file, summarizing the
change, and including a link back to the PR.

The format of

madanalysis5/madanalysis/UpdateNotes.txt

Lines 1 to 11 in 3fd09e7

    
           Update notes for MadAnalysis 5 (in reverse time order) 
        
           132  1.9   (2021/12/13) ma5team:    - Adding support for LLP (also in the SFS) [2112.05163]. 
        
                                               - Particle propagation module [2112.05163]. 
        
                                               - PYHF/simplified likelihood interface. 
        
                                               - TACO methods available. 
        
                                               - Python3 support. 
        
                                               - Connection of the PAD to the MA5 dataverse + reorganisation of how it works. 
        
                                               - Many minor bug fixes. 
        
                                               - Update to newer Delphes/Root versions

is confusing (what are the numbers in the first column)? The file also hasn't been updated since MadAnalysis5 got ported to GitHub, so I'm not sure if there is a format the dev team would like to keep.

When all the above are checked, delete everything above the dashed
line and fill in the pull request template.

Context:

Resolves #2

Description of the Change:

Add a Debian based Dockerfile that builds from the top level of the repository with the build context

docker build --file docker/Dockerfile --tag madanalysis5/madanalysis5:debug-local .

Add differential cross-section plot example (taken from http://madanalysis.irmp.ucl.ac.be/wiki/FAQNormalMode) as a test example to run after the build.
Add GitHub Actions workflow to build, test, and publish Docker images.

Benefits:

Dockerfiles and images are very powerful (build) testing and debugging tools and can be exceptional for giving users a sandbox area to play in and to use for demonstrations.

Possible Drawbacks:

PRs are like puppies. While I can help give guidance on how everything works, if this isn't something that the dev team is up for maintaining I can 100% appreciate and respect that.

Related GitHub Issues:

Issue #2.

matthewfeickert

@jackaraz I left some comments on this PR to make it easier for review, but before I actually mark it as such and flag you all for one, can you give me advice on the question RE: madanalysis/UpdateNotes.txt format?

matthewfeickert · 2022-01-19T04:21:46Z

docker/Dockerfile

@@ -0,0 +1,64 @@
+ARG BUILDER_IMAGE=python:3.9-slim-bullseye


This image doesn't include ROOT to make things smaller. If the dev team would prefer to have a base image with ROOT I can do that. Also, I'm assuming the dev team doesn't care if the base image is Debian or CentOS based, but if for some reason it does matter let me know.

Hi @matthewfeickert, thanks for all this! Having root would be great, that is generally the most problematic part for the users and our public analysis database mostly consist of Delphes based analyses for the time being so root is quite essential. The base is not so important ma5 has its own interface and the user doesn't need to know much about shell so we can handle the installation of the tools in the background. Also, you don't need to worry about the update notes I can take care of that, you are right it needs updating.

Having root would be great, that is generally the most problematic part for the users and our public analysis database mostly consist of Delphes based analyses for the time being so root is quite essential.

Okay cool. 👍 I'll do some revision and then rebase this PR with one of the ATLAS AMG lab images that contain ROOT (I maintain these to try to be decently small images, but you can be the judge on how big is "too big" for an image with madanalysis5).

matthewfeickert · 2022-01-19T04:22:22Z

docker/requirements.txt

+six>=1.16.0  # required by ma5
+scipy>=1.7.0  # optional for reinterpretation
+pyhf>=0.6.3  # optional for reinterpretation
+matplotlib>=3.5.0  # optional for histogramming


Including all of these as they are relatively lightweight dependencies.

this is perfect thanks I can add more as needed! also do you know if the latex and pdflatex compilers should be downloaded separately? I guess if you managed to execute the example they are already in the machine by default?

matthewfeickert · 2022-01-19T04:24:02Z

docker/Dockerfile

+    python -m piptools compile \
+        --generate-hashes \
+        --output-file /root/madanalysis5/requirements.lock \
+        /root/madanalysis5/docker/requirements.txt && \
+    python -m pip --no-cache-dir install --upgrade --requirement /root/madanalysis5/requirements.lock && \


Using piptools here to create a hash-level lockfile (/root/madanalysis5/requirements.lock) so that the Python environment that is shipped is fully reproducible and explained (piptools compile will add comments to the generated lockfile explaining why things were added).

matthewfeickert · 2022-01-19T04:25:10Z

examples/differential_xsec_plot.ma5

+# differential cross-section plot
+# c.f. http://madanalysis.irmp.ucl.ac.be/wiki/FAQNormalMode


The main motivation for adding this is so that the Docker image build in CI has something to test against, but it seems to be a simple example to add also.

matthewfeickert · 2022-01-19T04:26:09Z

examples/differential_xsec_plot.ma5

+install samples
+
+# load sample and set cross-section
+import /root/madanalysis5/samples/zz.lhe.gz as my_sample


This does have the disadvantage of being location specific, unless there is a way to import things starting from a relative path given by a shell variable.

Unfortunately, this depends on where you execute ma5 at the moment. The relative path is possible there is an anchor handle which can be set by

ma5> set main.currentdir = ...

this command is by default the execution location and everything can be added relative to this position. Once it is changed the relative paths must change as well. So for instance you executed from madanalysis5 folder

$ cd madanalysis5 $ ./bin/ma5 ma5> install samples ma5> import samples/zz.lhe.gz as my_sample

should work since it attaches currentdir anchor at the beginning of the path string by default.

matthewfeickert · 2022-01-19T04:28:09Z

docker/Dockerfile

+    python -m pip --no-cache-dir install --upgrade --requirement /root/madanalysis5/requirements.lock && \
+    python -m pip list && \
+    export PATH="$(find / -type d -iname madanalysis5)/bin:${PATH}" && \
+    python -c 'import multiprocessing; print(multiprocessing.cpu_count())' | ma5 && \


This works during the build, but when the image is run as a container later, ma5 will warn that

MA5: Checking the MadAnalysis 5 core library: MA5: => System configuration has changed since the last use. Need to rebuild the library.

I'm not sure if there is a way to set defaults that will be general enough that a Docker image user won't have to have this compile step happen every time. Thoughts?

I'll look into this. It's part of the legacy code; it has been built to be sensitive to any change.

matthewfeickert · 2022-01-19T04:29:34Z

.github/workflows/docker-debian.yml

+        run: |
+          DOCKER_IMAGE=madanalysis5/madanalysis5
+          VERSION=latest
+          MADANALYSIS_VERISON=v1.10.0


The version number will need to get bumped during releases (might want to look into things like bump2version for the whole project to make this easier/automatic).

Thanks @matthewfeickert I'll definitely look into this, it makes total sense to automatize the versioning.

matthewfeickert · 2022-01-19T04:30:45Z

.github/workflows/docker-debian.yml

+      # - name: Login to DockerHub
+      #   if: github.event_name != 'pull_request'
+      #   uses: docker/login-action@v1
+      #   with:
+      #     username: ${{ secrets.DOCKERHUB_USERNAME }}
+      #     password: ${{ secrets.DOCKERHUB_TOKEN }}


Publishing to DockerHub requires establishing an org and additional authentication. I left this in in case this is of interest in the near future, but this can easily get removed if people are fine with using GitHub Container Registry (ghcr) for the time being.

Build struture assumes running docker build from the top level of the repository. e.g. docker build --file docker/Dockerfile --tag madanalysis5/madanalysis5:debug-local . Add .dockerignore

Taken from http://madanalysis.irmp.ucl.ac.be/wiki/FAQNormalMode

Run tests builds on: * push events to main or tags * pull request events targetting main * on CRON schedule weekly at 01:23 UTC on Sundays * on releases * on demand with workflow dispatch Publish to the GitHub Container Registry by default, and publish with latest tag on pushed to main and publish with version tags on GitHub releases.

matthewfeickert commented Jan 19, 2022

View reviewed changes

jackaraz self-assigned this Jan 19, 2022

jackaraz added Docker regarding docker image ⚙️enhancement New feature or request labels Jan 19, 2022

matthewfeickert added 3 commits March 8, 2023 00:45

feat: Add Debian based Dockerfile

5e2120e

Build struture assumes running docker build from the top level of the repository. e.g. docker build --file docker/Dockerfile --tag madanalysis5/madanalysis5:debug-local . Add .dockerignore

feat: Add differential cross-section plot example

6086449

Taken from http://madanalysis.irmp.ucl.ac.be/wiki/FAQNormalMode

matthewfeickert force-pushed the feat/add-dockerfile-and-ci branch from 8a6a44b to 5ff8b5a Compare March 8, 2023 06:45

matthewfeickert mentioned this pull request Mar 8, 2023

Necessary dependencies to run full validation vs. validation with just ROOT #179

Open

BFuks mentioned this pull request May 31, 2023

lxplus computing environment? #200

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add Dockerfile and GitHub Actions based CI #12

feat: Add Dockerfile and GitHub Actions based CI #12

matthewfeickert commented Jan 19, 2022

matthewfeickert left a comment

matthewfeickert Jan 19, 2022

jackaraz Jan 19, 2022

matthewfeickert Jan 20, 2022

matthewfeickert Jan 19, 2022

jackaraz Jan 19, 2022

matthewfeickert Jan 19, 2022

matthewfeickert Jan 19, 2022

matthewfeickert Jan 19, 2022

jackaraz Jan 19, 2022

matthewfeickert Jan 19, 2022

jackaraz Jan 19, 2022

matthewfeickert Jan 19, 2022

jackaraz Jan 19, 2022

matthewfeickert Jan 19, 2022

	Update notes for MadAnalysis 5 (in reverse time order)

	132 1.9 (2021/12/13) ma5team: - Adding support for LLP (also in the SFS) [2112.05163].
	- Particle propagation module [2112.05163].
	- PYHF/simplified likelihood interface.
	- TACO methods available.
	- Python3 support.
	- Connection of the PAD to the MA5 dataverse + reorganisation of how it works.
	- Many minor bug fixes.
	- Update to newer Delphes/Root versions

		# differential cross-section plot
		# c.f. http://madanalysis.irmp.ucl.ac.be/wiki/FAQNormalMode

feat: Add Dockerfile and GitHub Actions based CI #12

Are you sure you want to change the base?

feat: Add Dockerfile and GitHub Actions based CI #12

Conversation

matthewfeickert commented Jan 19, 2022

Before submitting

matthewfeickert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment