feat: support other fields #135

vmx · 2022-03-07T18:36:57Z

It is now possible to use Neptune with Pasta curves on the GPU. In
order to make that possible, two new compile-time features are introduced.
bls to use BLS12-381 and pasta to use the Pasta curves (Pallas and
Vesta).

BREAKING CHANGE: The API now requires to pass in the field that
should be used. It does no longer default to BLS12-381.

If the GPU is used (enabled by the cuda and/or opencl features), you
now also need to specify the supported fields, bls and/or pasta.

vmx · 2022-03-07T18:42:45Z

src/batch_hasher.rs

+impl<
+        #[cfg(not(any(feature = "cuda", feature = "opencl")))] F: PrimeField,
+        #[cfg(any(feature = "cuda", feature = "opencl"))] F: PrimeField + GpuField,
+        A: Arity<F>,


@cryptonemo that's what I mentioned in our meeting. It's between crazy and nice.

vmx · 2022-03-08T13:55:04Z

I've converted it to a draft PR as the CUDA side of things needs more work. There's no need for a review atm.

vmx · 2022-03-10T12:41:56Z

CI is happy now, it's ready for review. This PR depends on zcash/pasta_curves#31.

cryptonemo · 2022-03-10T16:49:00Z

Cargo.toml

 criterion = "0.3"
 rand = "0.8.0"
 sha2 = "0.9"
 tempdir = "0.3"
 rand_xorshift = "0.3.0"
 serde_json = "1.0.53"
-pasta_curves = "0.2.1"
+pasta_curves = { git = "https://github.com/vmx/pasta_curves", branch = "ec-gpu" }


Needs updating before merge

cryptonemo · 2022-03-10T16:49:00Z

Cargo.toml

 criterion = "0.3"
 rand = "0.8.0"
 sha2 = "0.9"
 tempdir = "0.3"
 rand_xorshift = "0.3.0"
 serde_json = "1.0.53"
-pasta_curves = "0.2.1"
+pasta_curves = { git = "https://github.com/vmx/pasta_curves", branch = "ec-gpu" }


Needs updating before merge

porcuquine

Looks good, except for one naming issue (plus needs dependency update before merging).

gbench/src/main.rs

Cargo.toml

README.md

DrPeterVanNostrand · 2022-03-11T15:16:14Z

gbench/Cargo.toml

-blstrs = "0.4.0"
+blstrs = { version = "0.4.0", features = ["gpu"] }
+#pasta_curves = "0.3.0"
+pasta_curves = { git = "https://github.com/vmx/pasta_curves", branch = "ec-gpu", features = ["gpu"] }


Update before merging

gbench/src/main.rs

src/batch_hasher.rs

src/column_tree_builder.rs

src/proteus/gpu.rs

src/tree_builder.rs

DrPeterVanNostrand · 2022-03-11T15:39:36Z

Looks good!

vmx · 2022-03-11T16:34:19Z

I've rebased and addressed all review comments from @DrPeterVanNostrand in a separate commit.

DrPeterVanNostrand · 2022-03-24T17:53:53Z

@vmx ECC merged the blocking pasta_curves PR, so I think this is good to go once the dependencies are updated.

vmx · 2022-03-29T15:01:12Z

I've just rebased and squashed it into a single commit, no other code changes. This is ready to be merged. Remember that this is a breaking change.

vmx · 2022-03-29T16:10:17Z

@porcuquine you'll see a fil_pasta_curves dependency. This is just a temporary fork and we hope to have an upstream release at one point. The idea for this PR is to get it merged and then we use Neptune's master branch for rust-fil-proofs. We won't need a release for now. The hope is that there is a proper upstream release before we need a Neptune release. => we don't need a Neptune release, I'll let you know once we do.

porcuquine · 2022-03-30T16:40:24Z

@vmx How about we make a feature branch you can use as a stable Git dependency, so we can avoid having a Git dependency onmaster?

vmx · 2022-03-30T17:56:02Z

I find having it on master easier (you can also just use the specific commit), but whatever works. @DrPeterVanNostrand @cryptonemo what do you think? We can also just use this branch.

DrPeterVanNostrand · 2022-03-30T18:11:42Z

I'm fine with using this branch temporarily.

porcuquine · 2022-03-30T18:27:05Z

It's easier for the one consumer who needs it, but could be awkward if someone else needs something different — or if we need another release in-between, for whatever reason.

vmx · 2022-06-16T12:44:06Z

@porcuquine given that there's an upcoming breaking change release, it would be cool if this PR could be included. In now uses released versions of the dependencies, no other changes were made since the last review. I've rebased it on top of #150 and also changed the base branch to that for this PR, so that review is easier. Once that PR is merged, I'll again rebase it on master.

It is now possible to use Neptune with Pasta curves on the GPU. In order to make that possible, two new compile-time features are introduced. `bls` to use BLS12-381 and `pasta` to use the Pasta curves (Pallas and Vesta). BREAKING CHANGE: The API now requires to pass in the field that should be used. It does no longer default to BLS12-381. If the GPU is used (enabled by the `cuda` and/or `opencl` features), you now also need to specify the supported fields, `bls` and/or `pasta`.

So far tests were run in a single job for different configurations. Split those tests into separate jobs for more parllelism and hence shorter overall runtime.

vmx requested review from porcuquine and DrPeterVanNostrand March 7, 2022 18:36

vmx commented Mar 7, 2022

View reviewed changes

vmx force-pushed the fr-as-trait branch from 516fc0e to cb66a18 Compare March 7, 2022 18:48

vmx marked this pull request as draft March 8, 2022 13:54

vmx force-pushed the fr-as-trait branch from cb66a18 to d7c614c Compare March 9, 2022 19:00

vmx changed the base branch from master to field-in-fun-name March 9, 2022 19:01

vmx force-pushed the fr-as-trait branch 2 times, most recently from 0adcaeb to 3b91f48 Compare March 10, 2022 11:49

vmx changed the title ~~feat: use generic field everywhere~~ feat: support other fields Mar 10, 2022

vmx marked this pull request as ready for review March 10, 2022 12:40

cryptonemo reviewed Mar 10, 2022

View reviewed changes

porcuquine approved these changes Mar 11, 2022

View reviewed changes

gbench/src/main.rs Show resolved Hide resolved

vmx force-pushed the field-in-fun-name branch 2 times, most recently from 071aafc to 726f745 Compare March 11, 2022 13:45

Base automatically changed from field-in-fun-name to master March 11, 2022 14:27