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

Relax protobuf lower bound to 3.20. #15506

Merged
merged 3 commits into from
Apr 15, 2024

Conversation

bdice
Copy link
Contributor

@bdice bdice commented Apr 10, 2024

Description

This PR drops the lower bound of protobuf to 3.20, to make cuDF compatible with the versions used in Google Colab.

I tested this manually in Google Colab, which uses protobuf 3.20, and cuDF 24.02 seemed to work fine when reading ORC statistics (the only runtime feature in cuDF that needs protobuf). Note: cuDF 24.02 was built was a newer protobuf/protoc, version 4.x.

I will test this by forcing protobuf 3.20 in CI, and then revert those changes if tests pass.

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 Python Affects Python cuDF API. conda labels Apr 10, 2024
@github-actions github-actions bot added the ci label Apr 10, 2024
@bdice
Copy link
Contributor Author

bdice commented Apr 10, 2024

The test in 6dda780 went about like I expected. Wheels succeeded and conda ran into conflicts (specifically with pytorch, which is used for some cudf tests). Protobuf is used and pinned by more dependencies on the conda side, so I think it would be okay to relax the pinnings in dependencies.yaml (which would affect wheels) and leave the previous pinnings for conda recipes (meta.yaml). @beckernick Would that be a sufficient solution for Colab integration?

@github-actions github-actions bot removed the ci label Apr 10, 2024
@bdice bdice added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 10, 2024
@bdice bdice self-assigned this Apr 10, 2024
@bdice bdice requested a review from beckernick April 10, 2024 21:13
@beckernick
Copy link
Member

I tested this manually in Google Colab, which uses protobuf 3.20, and cuDF 24.02 seemed to work fine when reading ORC statistics (the only runtime feature in cuDF that needs protobuf). Note: cuDF 24.02 was built was a newer protobuf/protoc, version 4.x.

Nice! Thanks for doing this test. Perhaps a naive question, but are there any IO implications if the ORC file statistics are generated from a cuDF built against a protobuf major version that differs from the cuDF used to read the ORC file later?

@bdice
Copy link
Contributor Author

bdice commented Apr 10, 2024

Good question - I am pretty sure the answer is no, it doesn’t matter. This is only used for reading statistics, the writer is unaffected. If an old protobuf caused problems at runtime, I would expect it to show up somewhere in the cudf test suite. Everything passed so I think we’re okay.

@vyasr
Copy link
Contributor

vyasr commented Apr 11, 2024

See #15511 for my longer-term proposal (I have no objections to getting this PR in sooner).

@beckernick
Copy link
Member

I also tested the artifacts from this PR in Colab and saw no conflicts.

cudf-protobuf-pr-colab

@bdice bdice marked this pull request as ready for review April 15, 2024 15:41
@bdice bdice requested review from a team as code owners April 15, 2024 15:41
@bdice
Copy link
Contributor Author

bdice commented Apr 15, 2024

/merge

@rapids-bot rapids-bot bot merged commit ca7d85b into rapidsai:branch-24.06 Apr 15, 2024
70 checks passed
@bdice
Copy link
Contributor Author

bdice commented Apr 15, 2024

I confirmed that nightlies with this PR have no conflicts with Colab's pre-installed package versions.

!pip install --extra-index-url=https://pypi.anaconda.org/rapidsai-wheels-nightly/simple "cudf-cu12>=24.6.0a0,<=24.6"

image

bdice added a commit to bdice/cudf that referenced this pull request Apr 29, 2024
This PR drops the lower bound of protobuf to 3.20, to make cuDF compatible with the versions used in Google Colab.

I tested this manually in Google Colab, which uses protobuf 3.20, and cuDF 24.02 seemed to work fine when reading ORC statistics (the only runtime feature in cuDF that needs protobuf). Note: cuDF 24.02 was built was a newer protobuf/protoc, version 4.x.

I will test this by forcing protobuf 3.20 in CI, and then revert those changes if tests pass.

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

Approvers:
  - Nick Becker (https://github.com/beckernick)
  - Ray Douglass (https://github.com/raydouglass)
  - Matthew Roeschke (https://github.com/mroeschke)

URL: rapidsai#15506
raydouglass pushed a commit that referenced this pull request May 1, 2024
Backport of #15506 to cuDF 24.04.

Also backports #15574 to ignore a warning from newer cupy releases.

---------

Co-authored-by: Vyas Ramasubramani <vyasr@nvidia.com>
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 Python Affects Python cuDF API.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

5 participants