Skip to content

Update versions/recipe logic.#109

Merged
rapids-bot[bot] merged 8 commits intorapidsai:branch-0.35from
bdice:update-deps
Oct 24, 2023
Merged

Update versions/recipe logic.#109
rapids-bot[bot] merged 8 commits intorapidsai:branch-0.35from
bdice:update-deps

Conversation

@bdice
Copy link
Contributor

@bdice bdice commented Oct 20, 2023

Some improvements / modernizations for the package and dependencies. Depends on #108.

@bdice bdice requested review from a team as code owners October 20, 2023 21:18
"cupy-cuda11x",
]
"pytest-rerunfailures",
] # This list was generated by `rapids-dependency-file-generator`. To make changes, edit ../dependencies.yaml and run `rapids-dependency-file-generator`.
Copy link
Member

Choose a reason for hiding this comment

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

How did rapids-dependency-file-generator come up with this list? In particular the Dask/Distributed pins seem very arbitrary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are coming from dependencies.yaml. That file is supposed to be a "single source of truth" for all dependencies and their pinnings (with the exception of meta.yaml conda recipes and CMakeLists.txt which are not implemented at this time). Today, rapids-dependency-file-generator generates pyproject.toml, developer conda environments, and can generate dependency lists on-the-fly that aren't committed to the repo but are used during various builds (e.g. devcontainers, rapids-compose).

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 just noticed that conda/environments/ucxx-cuda118_arch-x86_64.yaml is not being generated by dependencies.yaml so I'll try to adjust that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are some features like channel pinnings (dask/label/dev::dask) and comments that would be lost in the developer environment. @pentschev Do you have thoughts on whether we should rework this to be auto-generated like other RAPIDS packages or would you prefer to maintain it separately?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See what you think of commit 4fc2c97. I added a dev section with the dependencies like libtool and autoconf and dask-cuda that aren't otherwise specified by ucxx as build/run dependencies.

Copy link
Member

Choose a reason for hiding this comment

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

These are coming from dependencies.yaml. That file is supposed to be a "single source of truth" for all dependencies and their pinnings (with the exception of meta.yaml conda recipes and CMakeLists.txt which are not implemented at this time). Today, rapids-dependency-file-generator generates pyproject.toml, developer conda environments, and can generate dependency lists on-the-fly that aren't committed to the repo but are used during various builds (e.g. devcontainers, rapids-compose).

Sorry, this was entirely my mistake. I knew it was coming from dependencies.yaml but I didn't realize it was being pinned to >=2023.1.1 there. I'm guessing when this was last adjusted python/pyproject.toml was not regenerated so I got confused by this seemingly arbitrary versioning.

Copy link
Member

Choose a reason for hiding this comment

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

There are some features like channel pinnings (dask/label/dev::dask) and comments that would be lost in the developer environment. @pentschev Do you have thoughts on whether we should rework this to be auto-generated like other RAPIDS packages or would you prefer to maintain it separately?

I guess that has been addressed by 4fc2c97, is that right? If so I think this question isn't relevant anymore.

See what you think of commit 4fc2c97. I added a dev section with the dependencies like libtool and autoconf and dask-cuda that aren't otherwise specified by ucxx as build/run dependencies.

I see that it included dask/label/dev as well which is great and addresses your question from above, these changes are perfect, thanks for doing that Bradley!

@jakirkham jakirkham closed this Oct 21, 2023
@jakirkham jakirkham reopened this Oct 21, 2023
@jakirkham jakirkham closed this Oct 21, 2023
@jakirkham jakirkham reopened this Oct 21, 2023
Copy link
Member

@pentschev pentschev left a comment

Choose a reason for hiding this comment

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

These changes are awesome, thanks @bdice for the work here. I've left a minor request to make versioning consistent with UCX-Py for the moment, let me know if you have thoughts.

Comment on lines +122 to +136
dev:
common:
- output_types: [conda]
packages:
# These packages are useful for development but not otherwise required to build/run
# RAPIDS
- pip
- dask-cuda==23.12.*
- dask-cudf==23.12.*
# UCX Build
- libtool
- automake
- autoconf
# UCXX Build
- pkg-config
Copy link
Member

Choose a reason for hiding this comment

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

Just to be sure I understand it correctly, the dev section is used to automatically generate conda/environments/*, is that it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The dev section is for dependencies that were added by hand to the previous conda environment (perhaps by you?) but do not fall into one of the other categories like build-time dependencies or run-time dependencies. I didn't want to lose that information but (if I understand correctly) none of the packages in dev are needed for CI to build and run tests.

"cupy-cuda11x",
]
"pytest-rerunfailures",
] # This list was generated by `rapids-dependency-file-generator`. To make changes, edit ../dependencies.yaml and run `rapids-dependency-file-generator`.
Copy link
Member

Choose a reason for hiding this comment

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

These are coming from dependencies.yaml. That file is supposed to be a "single source of truth" for all dependencies and their pinnings (with the exception of meta.yaml conda recipes and CMakeLists.txt which are not implemented at this time). Today, rapids-dependency-file-generator generates pyproject.toml, developer conda environments, and can generate dependency lists on-the-fly that aren't committed to the repo but are used during various builds (e.g. devcontainers, rapids-compose).

Sorry, this was entirely my mistake. I knew it was coming from dependencies.yaml but I didn't realize it was being pinned to >=2023.1.1 there. I'm guessing when this was last adjusted python/pyproject.toml was not regenerated so I got confused by this seemingly arbitrary versioning.

"cupy-cuda11x",
]
"pytest-rerunfailures",
] # This list was generated by `rapids-dependency-file-generator`. To make changes, edit ../dependencies.yaml and run `rapids-dependency-file-generator`.
Copy link
Member

Choose a reason for hiding this comment

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

There are some features like channel pinnings (dask/label/dev::dask) and comments that would be lost in the developer environment. @pentschev Do you have thoughts on whether we should rework this to be auto-generated like other RAPIDS packages or would you prefer to maintain it separately?

I guess that has been addressed by 4fc2c97, is that right? If so I think this question isn't relevant anymore.

See what you think of commit 4fc2c97. I added a dev section with the dependencies like libtool and autoconf and dask-cuda that aren't otherwise specified by ucxx as build/run dependencies.

I see that it included dask/label/dev as well which is great and addresses your question from above, these changes are perfect, thanks for doing that Bradley!

Copy link
Member

@pentschev pentschev left a comment

Choose a reason for hiding this comment

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

Thanks a lot @bdice !

@bdice
Copy link
Contributor Author

bdice commented Oct 24, 2023

/merge

@rapids-bot rapids-bot bot merged commit dc30ce5 into rapidsai:branch-0.35 Oct 24, 2023
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.

4 participants