Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

Docker builds for other CUDA versions, improve CI #4796

Merged
merged 10 commits into from
Nov 16, 2020

Conversation

epwalsh
Copy link
Member

@epwalsh epwalsh commented Nov 14, 2020

Feature request from our conversation here: #4793 (comment). This makes it so we'll build Docker images for both CUDA 10.2 and CUDA 11.0. These will be published as allennlp/allennlp:v1.2.1-cuda10.2 and allennlp/allennlp:v1.2.1-cuda11.0, respectively. We will still also provide a latest tag which for now will default to the CUDA 10.2 image for consistency with PyTorch.

This also merges our two GH Actions workflows (.github/workflows/master.yml and .github/workflows/pull_request.yml) into a single one: .github/workflows/ci.yml.

I still have a couple things to do:

  • Build our official images against different CUDA/torch versions, and push them to Docker hub with a tag that identifies the CUDA/torch version.
  • Test these images in our pull request CI. Maybe it's time to combine the pull_request.yml and master.yml workflows. Could just call it ci.yml.
  • Make similar changes to our image builds in allennlp-models.

@epwalsh epwalsh changed the title Docker builds for other CUDA versions Docker builds for other CUDA versions, improve CI Nov 16, 2020
@epwalsh epwalsh marked this pull request as ready for review November 16, 2020 23:22

AllenNLP releases Docker images to [Docker Hub](https://hub.docker.com/r/allennlp/) for each release. For information on how to run these releases, see [Installing using Docker](#installing-using-docker).

### Building a Docker image
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did this go away?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just moved it into the "Installing via Docker" section.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You did 🙈

# Only run this for releases on the main repo, not forks.
if: github.repository == 'allenai/allennlp' && github.event_name == 'release'
# Only run this for releases.
if: github.event_name == 'release'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did all of these if statements change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We checked if github.repository == 'allenai/allennlp' above, so it was redundant to have that again.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants