add full CI for wholegraph #58
Conversation
jameslamb
added
improvement
…ntally pulling in packages from the wholegraph repo
.github/workflows/build.yaml
Outdated
| @@ -0,0 +1,74 @@ | |||
| name: build | |||
There was a problem hiding this comment.
This workflow is the one that'll publish conda packages to rapidsai-nightly and wheels to https://pypi.anaconda.org/rapidsai-wheels-nightly/simple/.
I added this because it's there in #53 but @alexbarghi-nv ... are you all ready for that? If this PR is merged as-is, then both this repo and https://github.com/rapidsai/wholegraph will be publishing packages with the same names, and with an overlapping set of versions.
That's especially problematic because the wholegraph code here is out of sync with the wholegraph repo. For example, I saw some CI builds here picking up pylibwholegraph packages from the rapidsai-nightly channel instead of the artifacts from CI, resulting in failures like this:
pylibwholegraph 24.12.00a9 cuda12_py312_241018_g0efba33_9 rapidsai-nightly
...
AttributeError: module 'pylibwholegraph.binding.wholememory_binding' has no attribute 'determine_partition_plan'
Because the changes from rapidsai/wholegraph#194 appear not to have been ported over here.
I see 2 options:
- run nightly / branch builds in this repo, but don't turn on publishing yet
- merge this PR as-is, and then:
- bring the wholegraph code in this repo up to date with https://github.com/rapidsai/wholegraph
- move all wholegraph development to this repo (but keep the
wholegraphrepo un-archived for now, in case we need to produce 24.10 hotfixes) - have RAPIDS ops mark all other 24.12
pylibwholegraphandwholegraphconda packages onrapidsai-nightlyas broken
I think you should pursue Option 2, but you have a lot more context than I do about the plan for this repo and these libraries.
There was a problem hiding this comment.
We want Option 2. I'll reach out to the WholeGraph team.
There was a problem hiding this comment.
Actually, we should hold off on a decision. I forgot there's another discussion going on right now that needs to be completed first.
There was a problem hiding this comment.
should hold off on a decision
No problem.
What about removing the nightly workflows entirely (build.yaml and test.yaml) for now, and just limiting the scope of this work to getting CI fully working on PRs?
That'd allow us to keep making progress on setting up CI here, and as long as nothing's being published from here yet, it won't cause any conflicts for anyone depending on the packages being produced from the wholegraph / cugraph repos.
There was a problem hiding this comment.
That's fine for now. I'll let you know what happens with the other discussion.
There was a problem hiding this comment.
alright thanks! I just push a commit removing those nightly workflows. I'll put this up for review.
| "wheel", | ||
| "cmake>=3.26.4,!=3.30.0", | ||
| "cython>=3.0.0", | ||
| "ninja", |
There was a problem hiding this comment.
Don't worry about these changes for build dependencies of the cugraph-dgl and cugraph-pyg wheels... we're not building wheels yet for cugraph-pyg / cugraph-dgl on this repo yet anyway.
In the next PR after this, I'll update those libraries to rapids-build-backend and make the appropriate splits in dependencies.yaml.
jameslamb
changed the title
|
/merge |
…e dependency (#230) While working on adding CI for `wholegraph` in the new `cugraph-gnn` repo (rapidsai/cugraph-gnn#58), I noticed a few issues here. * `pylibwholegraph` imports `numpy` at runtime, but its wheels and conda package don't declare a runtime dependency on `numpy` * wheel runtime and testing dependencies are not being managed by `rapids-dependency-file-generator` * `wholegraph_binding` Cython code imports NumPy but doesn't use it * the wheel-testing CI environment is built up with a sequence of `pip install` calls instead of a single one to get all dependencies (see rapidsai/cugraph#4701) This proposes fixes for those things. ## Notes for Reviewers ### Where is the NumPy runtime dependency coming from? https://github.com/rapidsai/wholegraph/blob/0efba33835d6e4e104b5d7101a91e0ea55a6ca53/python/pylibwholegraph/pylibwholegraph/torch/data_loader.py#L14 Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Kyle Edwards (https://github.com/KyleFromNVIDIA) URL: #230
Another steps towards completing the work started in #53 Fixes #15 Contributes to rapidsai/build-planning#111 Proposes changes to get CI running on pull requests for `cugraph-pyg` and `cugraph-dgl` ## Notes for Reviewers Workflows for nightly builds and publishing nightly packages are intentionally not included here. See #58 (comment) Notebook tests are intentionally not added here... they'll be added in the next PR. Pulls in changes from these other upstream PRs that had not been ported over to this repo: * rapidsai/cugraph#4690 * rapidsai/cugraph#4393 Authors: - James Lamb (https://github.com/jameslamb) - Alex Barghi (https://github.com/alexbarghi-nv) Approvers: - Alex Barghi (https://github.com/alexbarghi-nv) - Bradley Dice (https://github.com/bdice) URL: #59
Continues the work of setting up CI here (breaking more changes off of #53). Proposes adding nightly builds / tests, but **not publishing** to the nightly PyPI index / `rapidsai-nightly` conda channel. Those publishing jobs should be added here only once `cugraph` folks are ready to fully move development of these libraries to this repo. ref: #58 (comment) This also removes some minor lingering cugraph references that look like they don't belong in this repo. Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Bradley Dice (https://github.com/bdice) URL: #61
This PR introduces full end-to-end CI for wholegraph.
Notes for Reviewers
Most of these changes were pulled from a mix of the following: