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

fix: upgrade to latest versions #1504

Merged
merged 2 commits into from
Sep 23, 2021
Merged

fix: upgrade to latest versions #1504

merged 2 commits into from
Sep 23, 2021

Conversation

vmx
Copy link
Contributor

@vmx vmx commented Sep 22, 2021

The latest versions of the gpu dependencies contain support for CUDA. Therefore
the gpu feature is split into cuda and opencl. By default only opencl is
enabled.

BREAKING CHANGE: The gpu feature is replaced by the opencl feature.

The latest versions of the gpu dependencies contain support for CUDA. Therefore
the `gpu` feature is split into `cuda` and `opencl`. By default only `opencl` is
enabled.

BREAKING CHANGE: The `gpu` feature is replaced by the `opencl` feature.
Copy link
Contributor

@dignifiedquire dignifiedquire left a comment

Choose a reason for hiding this comment

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

can you update the circle ci config to test both features please?

@vmx
Copy link
Contributor Author

vmx commented Sep 23, 2021

I thought I concentrate on OpenCL first as this is what we want to ship. Next step would then be CUDA with proper testing, updates of the README etc. So I would make the CUDA stuff a separate PR. Though perhaps it just works, so let's try it out.

@vmx
Copy link
Contributor Author

vmx commented Sep 23, 2021

I've added a new test run for the test_gpu_tree_building as it's the only test that runs on the GPU. It now runs on OpenCL and on CUDA.

@vmx
Copy link
Contributor Author

vmx commented Sep 23, 2021

@cryptonemo I leave the merge to you (I couldn't merge anyway due to "Required statuses must pass before merging").

@cryptonemo
Copy link
Collaborator

I also don't recall where the required CI tests are checked/unchecked. I'm ok merging this, but want to be sure that CI will not be broken on master from this point forward. Any insight?

@cryptonemo
Copy link
Collaborator

Oh wait, I think I found it: https://github.com/filecoin-project/rust-fil-proofs/settings/branch_protection_rules/2848255

If this seem correct (where I can uncheck ones after merging), I'll proceed. Sanity check?

@dignifiedquire
Copy link
Contributor

the required tests changed with the blst/pairing change from the earlier PR

@cryptonemo
Copy link
Collaborator

the required tests changed with the blst/pairing change from the earlier PR

I see, I've removed all of the old ones then.

@cryptonemo cryptonemo merged commit 2b4119e into master Sep 23, 2021
@vmx vmx deleted the update-gpu-deps branch September 24, 2021 07:35
@vmx vmx mentioned this pull request Sep 24, 2021
5 tasks
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.

3 participants