Skip to content

[skip-gpuci] Add dependencies.yaml for rapids-dependency-file-generator#3042

Merged
ajschmidt8 merged 11 commits intorapidsai:branch-23.02from
ChuckHastings:add_dependencies_yaml
Dec 9, 2022
Merged

[skip-gpuci] Add dependencies.yaml for rapids-dependency-file-generator#3042
ajschmidt8 merged 11 commits intorapidsai:branch-23.02from
ChuckHastings:add_dependencies_yaml

Conversation

@ChuckHastings
Copy link
Collaborator

First cut at dependencies.yaml.

We could certainly decompose this more, but wanted to get something out there quickly.

Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Thanks @ChuckHastings! I have some preliminary comments.

common:
- output_types: conda
packages:
- nvcc_linux-64=11.5
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need to split this by arch, since ARM doesn't use the same package name (it uses nvcc_linux-aarch64). See example here from cudf: https://github.com/rapidsai/cudf/blob/2048829ec66af1ceb7a9801702417d12ef13c77a/dependencies.yaml#L83-L94

Comment on lines +61 to +62
- clang=11.1.0
- clang-tools=11.1.0
Copy link
Contributor

@bdice bdice Dec 5, 2022

Choose a reason for hiding this comment

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

I think clang/clang-tools can be dropped unless you know a reason why not. Historically these were used only to provide clang-format, as I understand it, and we've been removing these packages from other repos as we migrate to dependencies.yaml. It's possible that some repos (cuML) wish to use clang-tools for running clang-tidy but I don't think that applies to cuGraph.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm OK with removing things. If we remove these from here, how does the C++ code formatting work? Is there some other mechanism we are using?

Copy link
Contributor

Choose a reason for hiding this comment

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

Many other RAPIDS repos have migrated to using pre-commit as a single source of truth for CI style checks. I will file a PR to update cugraph to match, since that's something we've been doing as part of the GitHub Actions migrations.

For clang-format specifically, switching to pre-commit lets us eliminate this section:

# Run clang-format and check for a consistent code format
CLANG_FORMAT=`python cpp/scripts/run-clang-format.py 2>&1`
CLANG_FORMAT_RETVAL=$?
ERRORCODE=$((ERRORCODE | ${CLANG_FORMAT_RETVAL}))

as well as the entire run-clang-format.py file in favor of a single hook definition like this example from cudf.

Then the style check script can also be simplified from ~79 lines to ~23 lines, like cudf.

- libcugraphops=23.02.*
- clang=11.1.0
- clang-tools=11.1.0
- boost
Copy link
Contributor

Choose a reason for hiding this comment

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

Where do we depend on boost?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like we used to depend on boost (circa version 0.6) but we may have eliminated that dependency. I'm testing without it now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removing boost

cpp_build:
common:
- output_types: conda
packages:
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's sort the contents of all packages: lists alphabetically.

- scipy
- networkx>=2.5.1
- scikit-build>=0.13.1
- python>=3.8,<3.10
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use a separate dependency list for Python versions. See cudf example: https://github.com/rapidsai/cudf/blob/2048829ec66af1ceb7a9801702417d12ef13c77a/dependencies.yaml#L190-L204

@BradReesWork BradReesWork added this to the 23.02 milestone Dec 6, 2022
@ajschmidt8 ajschmidt8 added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Dec 7, 2022
@ajschmidt8 ajschmidt8 changed the title [skip-ci] Add dependencies.yaml for rapids-dependency-file-generator [skip-gpuci] Add dependencies.yaml for rapids-dependency-file-generator Dec 7, 2022
ChuckHastings and others added 3 commits December 8, 2022 01:14
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
@ChuckHastings
Copy link
Collaborator Author

@bdice - addressed your comments, ready for more feedback.
@rlratzel - suggestions/thoughts? We talked about some other changes.

Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Thanks @ChuckHastings! This PR looks good enough to merge. We may have some additional tweaks as we work through GitHub Actions migrations, but this is a great starting point. Much appreciated!

@ChuckHastings ChuckHastings marked this pull request as ready for review December 8, 2022 17:28
@ajschmidt8 ajschmidt8 requested review from a team as code owners December 8, 2022 20:18
@ajschmidt8 ajschmidt8 requested a review from bdice December 8, 2022 20:20
@ajschmidt8
Copy link
Member

@ChuckHastings, I pushed some updates here. Let me know if you have any questions. It's mostly just some cleanup, but I also deleted the manually managed conda/environment files and replaced them with a file generated from dependencies.yaml. I only added the CUDA 11.5 variant and removed the old variants. That seems to be the convention we've gone with for other repositories.

Additionally I added a GitHub Action workflow to ensure that the generated files and source dependencies.yaml file stay in sync. This will be an optional GH check for now, but it will be required once we merge the subsequent GH Actions PR for cugraph.

@ajschmidt8 ajschmidt8 merged commit c1a56e2 into rapidsai:branch-23.02 Dec 9, 2022
@ChuckHastings ChuckHastings deleted the add_dependencies_yaml branch September 27, 2023 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants