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

Allow testing of earliest/latest dependencies. #1606

Closed
wants to merge 2 commits into from

Conversation

bdice
Copy link
Contributor

@bdice bdice commented Jul 11, 2024

Description

This PR allows the testing of earliest/latest dependencies. This is meant to be interpreted by each repository as it wishes, with CI scripts and rapids-dependency-file-generator to mediate which dependency versions are selected. For RMM, we are pinning to earliest supported numpy/numba versions.

See rapidsai/shared-workflows#228.

This is an experimental PR to prompt discussion / further work with @seberg.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@github-actions github-actions bot added the ci label Jul 11, 2024
@bdice bdice added feature request New feature or request non-breaking Non-breaking change labels Jul 11, 2024
@@ -12,7 +12,7 @@ rapids-logger "Create test conda environment"
rapids-dependency-file-generator \
--output conda \
--file-key test_python \
--matrix "cuda=${RAPIDS_CUDA_VERSION%.*};arch=$(arch);py=${RAPIDS_PY_VERSION}" | tee env.yaml
--matrix "cuda=${RAPIDS_CUDA_VERSION%.*};arch=$(arch);py=${RAPIDS_PY_VERSION};dependencies=${RAPIDS_DEPENDENCIES}" | tee env.yaml
Copy link
Contributor Author

@bdice bdice Jul 11, 2024

Choose a reason for hiding this comment

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

For wheel tests, we will have to engineer a way to use the oldest dependencies. We determine the test dependencies at build time, via [test] extras that are baked into the package. However, for testing earliest dependency pinnings, we really just want to add some extra pinnings to the environment at test time based on the value of RAPIDS_DEPENDENCIES. We may have to add a call in ci/test_wheel.sh that invokes rapids-dependency-file-generator to create a requirements.txt file that we install after installing the previously-built wheel.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, that makes sense to me (and it is really cool to see how straight forward this is looking like).
Don't want to mess with the builds, just testing, so branching between using package[test] and a custom requirements install on RAPIDS_DEPENDENCIES="earliest" seems good.

- output_types: [conda, requirements, pyproject]
matrices:
- matrix:
dependencies: "earliest"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This worked as desired:

RAPIDS logger » [07/11/24 17:12:53]
┌─────────────────────────────────────┐
|    Create test conda environment    |
└─────────────────────────────────────┘

# This file is generated by `rapids-dependency-file-generator`.
# To make changes, edit ../../dependencies.yaml and run `rapids-dependency-file-generator`.
channels:
- rapidsai
- rapidsai-nightly
- conda-forge
dependencies:
- cuda-version=11.8
- numba==0.57.*
- numpy==1.23.*
- pytest
- pytest-cov
- python=3.9
name: test_python_cuda-118_arch-x86_64_py-39_dependencies-earliest

...

numba                     0.57.1           py39hb75a051_0    conda-forge
numpy                     1.23.5           py39h3d75532_0    conda-forge

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. An optional suggestion for terminology: maybe "oldest" or "minimum" instead of "earliest"?
  2. Not sure if any RAPIDS libraries support optional dependencies, but one would be able to both required and optional dependencies here?
  3. Speculating a use case: I would imagine for a particular common block, a project would ideally want to test both the latest and earliest dependencies so would a specification like:
    common:
      - output_types: [conda, requirements, pyproject]
      - version_types: [latest, earliest]

make sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have an opinion on naming, the name "earliest" already exists in the common workflow (but not sure it is actually used for much there).

earliest/latest is only really relevant for testing, so I think we don't need any of the other things mentioned and this is just fine.
I.e. all we need is a way to generate two variations for the testing step, which is already fully separated from the build step.

Copy link
Contributor

Choose a reason for hiding this comment

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

Needed a bit to realize that a currently valid way to specify say that something works for say Python 3.12 and 3.13 is 3.1[23] fromn the fnmatch synatx.

Likely not often needed in rapids, but it occurred to me that minimal versions can often be Python version dependent.
(It's unfortunate that conda/pip have no tool to ask to resolve the oldest versions.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In RAPIDS we have historically chosen lower bounds that work for all RAPIDS-supported Python versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I chose “earliest/latest” because we named our CUDA drivers that way in shared-workflows. There is also precedent for “oldest” in packages like “oldest-supported-numpy.” Please feel free to change it — I have bequeathed these pull requests to @seberg.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a very weak preference to avoid calling it “minimum” versions because we usually say a package with few dependencies or no optional dependencies has “minimal dependencies.”

rapids-bot bot pushed a commit that referenced this pull request Aug 22, 2024
Mostly identical to gh-1606, which allows the testing of oldest/latest dependencies.  What oldest/latest dependencies means exactly has to be set by each project in their `dependencies.yaml` file for the test env.

See rapidsai/shared-workflows#228 for the workflow changes.

This is part of preparing for 2.0 (which should just work, but in the end no need to include it in the same PR).

*Modifications from draft PR:*
- I renamed it to `oldest`, as suggested by Matthew.
- Noticed that the name is different between wheel and conda workflows, so modified both to include all matrix information.

(Draft, since the shared workflow should be pushed in first to revert the branch renaming probably.  Need to test that the wheel workflow is correct.)

Authors:
  - Sebastian Berg (https://github.com/seberg)
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - James Lamb (https://github.com/jameslamb)
  - https://github.com/jakirkham

URL: #1613
@bdice
Copy link
Contributor Author

bdice commented Oct 31, 2024

@seberg Do you have interest in working on this, or should we close it?

@bdice
Copy link
Contributor Author

bdice commented Oct 31, 2024

Ah, nevermind! This is superseded by #1613. Apologies. I will close.

@bdice bdice closed this Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci feature request New feature or request non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants