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

Add CI jobs to upload nightly wheels to Anaconda #3945

Draft
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

agriyakhetarpal
Copy link
Member

@agriyakhetarpal agriyakhetarpal commented Mar 30, 2024

Description

Note

For some of the follow-ups to and requirements for this PR, please read through the thread on #3251

This PR configures the publish_pypi.yml workflow (maybe we should rename it to publish_wheels.yml now?) to add a job that uploads wheels to Anaconda. Currently, wheels will be uploaded on the same cadence as the workflow, so they are more like "fortnightlies" rather than nightlies.

Fixes #3251

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

  • New feature (non-breaking change which adds functionality)

Key checklist:

  • No style issues: $ pre-commit run (or $ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)
  • All tests pass: $ python run-tests.py --all (or $ nox -s tests)
  • The documentation builds: $ python run-tests.py --doctest (or $ nox -s doctests)

You can run integration tests, unit tests, and doctests together at once, using $ python run-tests.py --quick (or $ nox -s quick).

Further checks:

  • Code is commented, particularly in hard-to-understand areas

Comment on lines +216 to +219
publish_anaconda:
# This job is only of value to PyBaMM and would always be skipped in forks
# It is meant to run on a schedule, when manually triggered, and does not
# run on releases.
Copy link
Member Author

@agriyakhetarpal agriyakhetarpal Mar 30, 2024

Choose a reason for hiding this comment

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

Note: this job is configured differently and is isolated from the PyPI upload job for the reason that the latter uses the pypi repository environment, which must not be accessed by other jobs (and also that GitHub Actions currently does not allow configuring an environment to a step inside a job).

Comment on lines +231 to +234
- name: Move all package files to files/
run: |
mkdir files
mv windows_wheels/* linux_wheels/* macos_amd64_wheels/* macos_arm64_wheels/* files/
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: the source distribution is not uploaded to Anaconda because it does not make sense to do so. The nightly uploads are supposed to be binary-only. It is therefore not copied into the files/ staging area for upload.

.github/workflows/publish_pypi.yml Outdated Show resolved Hide resolved
mv windows_wheels/* linux_wheels/* macos_amd64_wheels/* macos_arm64_wheels/* files/

- name: Push to Anaconda PyPI index
uses: scientific-python/upload-nightly-action@b67d7fcc0396e1128a474d1ab2b48aa94680f9fc # v0.5.0
Copy link
Member Author

@agriyakhetarpal agriyakhetarpal Mar 30, 2024

Choose a reason for hiding this comment

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

It's recommended to pin this action to a commit hash. I think we should do this for all of our other actions in other jobs too as a security measure (Dependabot will update both the hash and the version comment using regex).

@agriyakhetarpal
Copy link
Member Author

agriyakhetarpal commented Mar 30, 2024

This is still a draft because I need to add instructions for how to let people use these nightly wheels in the installation guide. I think I can prepare that locally, but it would make more sense to push those changes when we will have the repository secrets and the custom index configured for uploads (and subsequently downloads). I have shared the details for the procedure with Valentin.

@kratman
Copy link
Contributor

kratman commented Apr 1, 2024

Do we have upload limits on anaconda? Nightly builds would make a lot of releases

@agriyakhetarpal
Copy link
Member Author

Do we have upload limits on anaconda? Nightly builds would make a lot of releases

Yes, there is a 5 GB upload limit, but the action handles deleting releases older than N versions or M days automatically if one pushes multiple versions (N and M can be configured). I forgot to mention here that since we are not configuring any SCM-based versioning for the nightlies, they will be overwritten on each upload and we will not hit any limits.

This is what I had sent to @tinosulzer the other day about the requirements here:

  1. A nightly channel to push wheels to, to be filled in anaconda_nightly_upload_organization
  2. An API key generated to push to this channel and then added as a repository secret
  3. A manual upload for a single wheel to this channel for the first time (any platform/architecture shall work, we can delete it later).

@agriyakhetarpal
Copy link
Member Author

Ready for review now with a preliminary draft of the instructions, all we will need is to update the link when it is ready.

@agriyakhetarpal agriyakhetarpal marked this pull request as ready for review April 2, 2024 18:26
Copy link

codecov bot commented Apr 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.58%. Comparing base (cf09d5e) to head (25da440).
Report is 250 commits behind head on develop.

Current head 25da440 differs from pull request most recent head 3d922d0

Please upload reports for the commit 3d922d0 to get more accurate results.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #3945   +/-   ##
========================================
  Coverage    99.58%   99.58%           
========================================
  Files          257      257           
  Lines        21249    21249           
========================================
  Hits         21161    21161           
  Misses          88       88           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@arjxn-py
Copy link
Member

arjxn-py commented Apr 3, 2024

I forgot to mention here that since we are not configuring any SCM-based versioning for the nightlies, they will be overwritten on each upload and we will not hit any limits.

Do we plan to configure any SCM-based versioning?

@agriyakhetarpal
Copy link
Member Author

Do we plan to configure any SCM-based versioning?

Not for now until we migrate to a new build system. If we were to set up setuptools-scm and then we start removing setuptools soon, it's going to cause issues with file inclusion and exclusion later, because scikit-build-core uses different ways to handle that and we would need to add something like versioneer and go through it all over again.

What I will suggest for this right now is to use local version identifiers as noted in PEP 440 to distinguish between PyPI releases and these nightlies, and therefore rename the wheel to add +dev after the version tag and before the platform and ABI tags, similar to what JAX does for its GPU wheels. I'll push another commit to make this change.

@agriyakhetarpal
Copy link
Member Author

agriyakhetarpal commented Apr 8, 2024

Bumping the version for built wheels would require a bit of restructuring in the workflow to update the version before building the nightlies and that means this should probably be done when we have some SCM-based versioning. I think we can go ahead with this workflow right now and (I merged the latest changes from develop just now).

The MO to test the nightlies is described in SPEC-0004, which means that PyBOP as a dependent will be implementing a weekly workflow which downloads the PyBOP and PyBaMM nightly wheels as such, something like:

      - name: Test nightly builds
        run: |
          pip install pybop pybamm --pre --extra-index-url https://pypi.anaconda.org/pybamm-and-friends/simple
          pytest --pyargs pybop  # or any other test invocation command

(cc: @BradyPlanden), and a subsequent step/job can then open an issue for PyBOP about incoming failures due to changes in PyBaMM. It is to be noted that the use of --extra-index-url here rather than --index-url means that we open up indices such as PyPI to search in for the case of downloading PyBaMM's dependencies (NumPy, SciPy, CasADi, etc.) because their wheels would not exist in our own nightly index. If it is needed (which IMO it isn't), we can specify multiple index URLs to test in CI and add the https://pypi.anaconda.org/scientific-python-nightly-wheels/simple index.

Users should also look to use --extra-index-url though, because PyBaMM doesn't have a lot of dependents right now (four, based on GitHub's insights). I probably will have to update the installation guide about this so that they are not misled with not being able to download scipy/numpy/casadi/xarray from our index, but still warn them enough that they should use these command-line flags carefully and at their own discretion.

Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>
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.

Infrastructure for nightly releases
3 participants