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

feat: send sync region #180

Merged

Conversation

alexander-camuto
Copy link

@alexander-camuto alexander-camuto commented May 8, 2023

As discussed in #173, layouts for large circuits (100mil+) parameters can be quite time consuming.

Here we make the Region struct both Send and Sync so as to enable parallel layouts. We do so principally by constraining the inner element of Region (region: &'r mut dyn layouter::RegionLayouter<F>,) to be Send and Sync

pub trait RegionLayouter<F: Field>: fmt::Debug + Send + Sync { // add the new constraints here
    /// Enables a selector at the given offset.
...

Note: the copy constraint labels can be non-deterministic in the case of a parallelized layout. In particular, referring to step 2 of the algorithm in the Halo2 Book for permutation arguments:

  1. Otherwise, left and right belong to different cycles. Make left the larger cycle and right the smaller one, by swapping them iff sizes(aux(left))<sizes(aux(right)).

The smaller cycle in a multi-threaded setting can be non-deterministic and as such, the constructed copy constraint labels can differ between the key generation and proof stage. A way to mitigate this is to make this label deterministic. What I've done in zkonduit@5eebade is to swap:

if self.sizes[left_cycle.0][left_cycle.1] < self.sizes[right_cycle.0][right_cycle.1] {
std::mem::swap(&mut left_cycle, &mut right_cycle);
}

for a deterministic label propagation such as:

if right_cycle.0 >= left_cycle.0 && right_cycle.1 > left_cycle.1 {
            std::mem::swap(&mut left_cycle, &mut right_cycle);
}

The downside is that sometimes this means that the re-labelling of the right cycle can take more time (as its no longer necessarily the smaller cycle).

Given the potential impacts to the single threaded case we have remaining TODOs:

  • make the cycle label propagation changes conditional on a parallel-region feature flag (such that when deactivated the cycle label propagation is as before)
  • add some unit tests that leverage parallelism (rather than relying on the https://github.com/zkonduit/ezkl tests).

Copy link
Member

@CPerezz CPerezz left a comment

Choose a reason for hiding this comment

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

I'd change the feature flag name to something like: parallel-synthesis or similar.
Also, as mentioned, would add unit-tests to check that with a ton of small regions assigned in parallel we get the safety we expect.

Finally, (if possible) I'd add benchmarks and/or an example. I think with this the PR/feature addition can be accepted!
We would appreciate it so much!

@alexander-camuto
Copy link
Author

alexander-camuto commented May 10, 2023

Quick note. Had to reconfigure some of the Assembly struct used for creating permutation arguments. The current algorithm is currently non-deterministic in a multi-threaded setting. In particular though the cycles that are created have the same constituent elements, the ordering within that cycle can differ.

For an illustration of how the order in which the copy constraints are generated can create different cycle directions see:

Untitled-2022-01-06-1700(6)

To mitigate this we replace the order-dependent Vec used in the cycle construction algorithm with order-invariant BTreeSet

/// Struct that accumulates all the necessary data in order to construct the permutation argument.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Assembly {
    /// Columns that participate on the copy permutation argument.
    columns: Vec<Column<Any>>,
    /// Mapping of the actual copies done.
    cycles: Vec<usize, BTreeSet<(usize, usize)>>,
    /// Mapping of the actual copies done.
    aux: HashMap<(usize, usize), usize>,
    /// total length of a column
    col_len: usize,
    /// number of columns
    num_cols: usize,
}

The algorithm for constructing the cycles is then (for a copy constraint between two elements a and b)

  1. Get cycle for a and b using aux (this is None if they are singletons not part of a cycle).
  2. If they are part of the same cycle return.
  3. Get the size of each element's cycle using the length of the cycles Vec entry (=1 if the cycle index is None)
  4. Pick the smallest cycle to merge into the larger one
  5. Merging cycles is as simple as:
    • Copy cycles entry for the smaller cycle into the larger cycle entry (it will do the sorting for us).
    • Remove entry for the smaller cycle.
  6. Relabel aux to point to the new cycle index for the (ex)smaller cycle element.

The constructed cycles are then:

  • each element of the BTreeSet points to the next element in the set
  • the last element of the set points to the first element.

Note: Constant assignment, if done inside a parallel loop, will still be non-deterministic so we should add a warning somewhere against this

@alexander-camuto
Copy link
Author

There's a chance that even in the single threaded case this is more efficient than the existing cycle creating algo -- as we don't store the entire matrix of size (num_cols * num_rows), only the elements that are part of a cycle.

@alexander-camuto
Copy link
Author

Have added a small elementwise vector multiplication example (single threaded and multithreaded). To time the (mock) proving time for using the new mapping cycle algorithm:

cargo run --example vector-mul --features="thread-safe-region"

With the current algorithm

cargo run --example vector-mul 

To run the mock prover in parallel:

cargo run --example vector-mul-parallel-synth --features="thread-safe-region"

@alexander-camuto
Copy link
Author

  • Have added benchmarks as well
cargo bench --bench plonk-vec
cargo bench --bench plonk-vec --features thread-safe-region 
cargo bench --bench plonk-vec-parallel-synth --features thread-safe-region
  • (timed) examples (see above). @CPerezz think its ready to review :)

@alexander-camuto alexander-camuto marked this pull request as ready for review May 16, 2023 15:26
Copy link

@han0110 han0110 left a comment

Choose a reason for hiding this comment

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

Overall LGTM! For most comments left just want to save the lines of code so we could have less effort on maintaining the library, hope it make sense!

halo2_proofs/examples/vector-mul.rs Show resolved Hide resolved
halo2_proofs/benches/plonk-vec.rs Outdated Show resolved Hide resolved
halo2_proofs/Cargo.toml Outdated Show resolved Hide resolved
halo2_proofs/src/plonk/permutation/keygen.rs Outdated Show resolved Hide resolved
halo2_proofs/src/plonk/permutation/keygen.rs Outdated Show resolved Hide resolved
halo2_proofs/src/plonk/permutation/keygen.rs Show resolved Hide resolved
halo2_proofs/src/plonk/permutation/keygen.rs Outdated Show resolved Hide resolved
Copy link

@han0110 han0110 left a comment

Choose a reason for hiding this comment

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

All LGTM now! Thanks for addressing all the comments!

@daira
Copy link

daira commented May 25, 2023

The downside is that sometimes this means that the re-labelling of the right cycle can take more time (as its no longer necessarily the smaller cycle).

Note that there are other algorithms that could be used to generate the cycles. As pointed out by @defuse, what we need to represent the sets of equality-constrained cells is a https://en.wikipedia.org/wiki/Disjoint-set_data_structure . The algorithm we used was designed for simplicity, and there are alternatives that might be more suited to parallelism.

For example:

In 1994, Richard J. Anderson and Heather Woll described a parallelized version of Union–Find that never needs to block.[8]

@daira
Copy link

daira commented May 25, 2023

Note that if you:

  • use any, potentially nondeterministic, algorithm to find the disjoint sets;
  • then in a separate phase —parallelised across sets— sort each set and construct the corresponding cycle

then that's almost certainly faster than maintaining sortedness during the set finding (which is what BTreeSet is doing). This is because the algorithm used to find the sets is less constrained, and because it will involve less copying — so fewer, and more local, memory accesses.

@alexander-camuto
Copy link
Author

@daira thank you for the detailed review. I've made it such that ordering is not tracked during the cycle creation. We order once at the end in parallel across cycles.

@alexander-camuto
Copy link
Author

heya @CPerezz lmk if there's anything else needed here :)

Copy link
Member

@CPerezz CPerezz left a comment

Choose a reason for hiding this comment

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

This looks really nice! We could file an issue (in case you consider it can be useful) regarding Daira's comment about other algorithms to create cycles.

In any case, the PR looks good to be merged!

Thank you so much for your work!!

@CPerezz CPerezz merged commit a764a7f into privacy-scaling-explorations:main Jun 1, 2023
@CPerezz CPerezz mentioned this pull request Jun 1, 2023
@alexander-camuto alexander-camuto deleted the ac/send-sync-region branch June 1, 2023 19:55
@alexander-camuto alexander-camuto restored the ac/send-sync-region branch June 2, 2023 14:17
jonathanpwang added a commit to axiom-crypto/halo2 that referenced this pull request Aug 15, 2023
* - Implements `PartialOrd` for `Value<F>`
- Adds a `transpose` method to turn `Value<Result<_>>` into
  `Result<Value<_>>`
- `Expression::identifier()` remove string memory reallocation

* Fix MockProver `assert_verify` panic errors (privacy-scaling-explorations#118)

* fix: Support dynamic lookups in `MockProver::assert_verify`

Since lookups can only be `Fixed` in Halo2-upstream, we need to add
custom suport for the error rendering of dynamic lookups which doesn't
come by default when we rebase to upstream.

This means that now we have to print not only `AdviceQuery` results to
render the `Expression` that is being looked up. But also support
`Instance`, `Advice`, `Challenge` or any other expression types that are
avaliable.

This addresses the rendering issue, renaming also the `table_columns`
variable for `lookup_columns` as the columns do not have the type
`TableColumn` by default as opposite to what happens upstream.

* fix: Don't error and emit empty String for Empty queries

* feat: Add `assert_sarisfied_par` fn to `MockProver`

* fix: Address clippy errors

* chore: Address review comments

* chore: Fix clippy lints

Resolves: privacy-scaling-explorations#116

* Remove partial ordering for value

* Remove transpose

* Parallelize SHPLONK multi-open prover (privacy-scaling-explorations#114)

* feat: parallelize (cpu) shplonk prover

* shplonk: improve `construct_intermediate_sets` using `BTreeSet` and
`BTreeMap` more aggressively

* shplonk: add `Send` and `Sync` to `Query` trait for more parallelization

* fix: ensure the order of the collection of rotation sets is independent
of the values of the opening points

Co-authored-by: Jonathan Wang <[email protected]>

* fix: FailureLocation::find empty-region handling (privacy-scaling-explorations#121)

After working on fixing
privacy-scaling-explorations/zkevm-circuits#1024, a bug was found in the
verification fn of the MockProver which implies that while finding a
FailureLocation, if a Region doesn't contain any rows.

This is fixed by introducing a 2-line solution suggested by @lispc.

Resolves: privacy-scaling-explorations#117

* Feature: Expose Fixed columns & Assembly permutation structs in MockProver instance (privacy-scaling-explorations#123)

* feat: Expose fixed columns in MockProver

* change: Make `Assembly` object public & add getters

* chore: Address leftover TODOs

* Feature to serialize/deserialize KZG params, verifying key, and proving key into uncompressed Montgomery form (privacy-scaling-explorations#111)

* feat: read `VerifyingKey` and `ProvingKey` does not require `params` as
long as we serialize `params.k()`

* feat: add features "serde-raw" and "raw-unchecked" to
serialize/deserialize KZG params, verifying key, and proving key
directly into raw bytes in internal memory format.
So field elements are stored in Montgomery form `a * R (mod p)` and
curve points are stored without compression.

* chore: switch to halo2curves 0.3.1 tag

* feat: add enum `SerdeFormat` for user to select
serialization/deserialization format of curve and field elements

Co-authored-by: Jonathan Wang <[email protected]>

* Add support for Column annotations for MockProver debugging (privacy-scaling-explorations#109)

* feat: Add `name_column` to `Layouter` & `RegionLayouter`

This adds the trait-associated function `name_column` in order to enable
the possibility of the Layouter to store annotations aobut the colums.

This function does nothing for all the trait implementors (V1,
SimpleFloor, Assembly....) except for the `MockProver`. Which is
responsible of storing a map that links within a `Region` index, the
`column::Metadata` to the annotation `String`.

* feta: Update metadata/dbg structs to hold Col->Ann mapping

* feat: Update emitter module to print Column annotations

* feat: Add lookup column annotations

This adds the fn `annotate_lookup_column` for `ConstraintSystem` which
allows to carry annotations for the lookup columns declared for a
circuit within a CS.

* feat: Add Lookup TableColumn annotations

This allows to annotate lookup `TableColumn`s and print it's annotation
within the `assert_satisfied` fn.

This has required to change the `ConstraintSystem::lookup_annotations`
to have keys as `metadata::Column` rather than `usize` as otherwise it's
impossible within the `emitter` scope to distinguish between regular
advice columns (local to the Region) and fixed columns which come from
`TableColumn`s.

* fix: Customly derive PartialEq for metadata::Region

This allows to ignore the annotation map of the metadata::Region so that
is easier to match against `VerifyFailure` errors in tests.

* fix: Update ConstraintNotSatisfied testcase

* fix: Update Debug & Display for VerifyFailure

It was necessary to improve the `prover.verify` output also. To do so,
this required auxiliary types which are obfuscated to any other part of the lib
but that are necessary in order to be able to inject the Column names inside of
the `Column` section itself.
This also required to re-implement manually `Debug` and `Display` for this enum.

This closes zcash#705

* fix: Address clippy & warnings

* fix: Add final comments & polish

* fix: Resolve cherry-pick merge conflics & errors

* chore: Change DebugColumn visibility

* chore: Allow to fetch annotations from metadata

* chore: Fix clippy lints

* chore: Remove comments from code for testing

* feat: Add support for Advice and Instance anns in lookups

* feat: Allow `V1` layouter to annotate columns too

* fix: Support `Constant` & `Selector` for lookup exprs

* chore: Address review comments

* chore: Propagete write! result in `VerifyFailure::Display`

* chore: Address clippy lints

* chore: Move Codecov, wasm-build, Bitrot & doc-tests to push (privacy-scaling-explorations#125)

* chore: Move Codecov, wasm-build, Bitrot & doc-tests to push

This should cut down significantly the CI times on every push done to a
branch for a PR.
Resolves: privacy-scaling-explorations#124

* chore: Add back `push` on CI checks

* fix: Allow to compare `Assembly` structs (privacy-scaling-explorations#126)

This was missing in privacy-scaling-explorations#123 so this PR fixes it.

* Add keccak256 hasher for transcript (#2)

* Add keccak256 hasher for transcript

* Fix keccak256 common point prefix

* Remove unnecessary hasher_* variables

* fix: transcript instantiation in poseidon benchmark loop (privacy-scaling-explorations#128)

* Improve performance of vk & pk keygen and of default `parallelize` chunking size (privacy-scaling-explorations#127)

* Squashed commit of the following:

commit 17e3c4e
Author: Mickey <[email protected]>
Date:   Fri Jul 15 11:10:32 2022 +0800

    speed up generate vk pk with multi-thread

* fix

* Improve performance of vk & pk keygen and of default `parallelize` chunking size.

Reduces proving time on large circuits consistently >=3%.
Builts upon [speed up generate vk pk with multi-thread](privacy-scaling-explorations#88)
Fixes: privacy-scaling-explorations#83

* fix: Force `VerifyFailure` to own the annotations map (privacy-scaling-explorations#131)

* fix: Force `VerifyFailure` to own the annotations map

Since otherwise we can't move the `VerifyFailure` vec's confortably, and
also, we're required to have a lot of lifetime annotations, it was
decided to force the `VerifyFailure` to own the Annotation maps.

This shouldn't be too harmful as it only triggers when testing.

Resolves: privacy-scaling-explorations#130

* chore: Address clippy lints

* feat: call synthesize in `MockProver` multiple times to behave same as real prover

* feat: check advice assignment consistency between different phases

* fix: Support annotations for CellNotAssigned in verify_par (privacy-scaling-explorations#138)

* feat: Add `assert_satisfied_at_rows_par` variant (privacy-scaling-explorations#139)

Resolves: privacy-scaling-explorations#133

* Expose mod `permutation` and re-export `permutation::keygen::Assembly` (privacy-scaling-explorations#149)

* feat: expose mod ule `permutation` and re-export `permutation::keygen::Assembly`

* feat: derive `lone` for `permutation::keygen::Assembly`

* feat: bump MSRV for `inferno`

* feat(MockProver): replace errors by asserts

In MockProver, replace all code that returns an error by an assert that panics instead of returning the error.  This change aims to make it easier to debug circuit code bugs by getting backtraces.

* MockProver test utililities (privacy-scaling-explorations#153)

* test/unwrap_value: escape Value safety in the dev module

* test/mock-prover-values: MockProver exposes the generated columns to tests

* test/mock-prover-values: doc

* mockprover-util: remove unwrap_value

---------

Co-authored-by: Aurélien Nicolas <[email protected]>

* feat: Parallel random blinder poly impl (privacy-scaling-explorations#152)

* feat: Parallelize `commit` blinder poly generator method

Solves the concerns raised in privacy-scaling-explorations#151 related to the performance of the
random poly generator inside of `commit`.

Resolves: privacy-scaling-explorations#151

* chore: add `from_evals` for Polynomial

* chore: add benches for commit_zk serial vs par

* fix: Correct thread_seeds iter size

* fix: Clippy

* chore: apply review suggestions

* fix: Inconsisten num of Scalars generated parallely

This fix from @ed255 fixes an error on the code proposal which was
rounding the num of Scalars to be generated and so, was producing
failures.

Co-authored-by: Edu <[email protected]>

* remove: legacy comments & code

---------

Co-authored-by: Edu <[email protected]>

* change: Migrate workspace to pasta_curves-0.5 (privacy-scaling-explorations#157)

* change: Migrate workspace to pasta_curves-0.5

This ports the majority of the workspace to the `pasta_curves-0.5.0`
leaving some tricky edge-cases that we need to handle carefully.

Resolves: privacy-scaling-explorations#132

* fix: Complete latest trait bounds to compile halo2proofs

* change: Migrate examples & benches to pasta 0.5

* change: Migrate halo2_gadgets to pasta-0.5

* change: Update gadgets outdated code with latest upstream

* fix: Sha3 gadget circuit

* fix: doc tests

* chore: Update merged main

* fix: Apply review suggestions

* fix: pin `halo2curves` version to `0.3.2`

* Extend Circuit trait to take parameters in config (privacy-scaling-explorations#168)

* Extend Circuit trait to take parameters in config

The Circuit trait is extended with the following:
```
pub trait Circuit<F: Field> {
    /// [...]
    type Params: Default;

    fn params(&self) -> Self::Params {
        Self::Params::default()
    }

    fn configure_with_params(meta: &mut ConstraintSystem<F>, params: &Self::Params) -> Self::Config {
        Self::configure(meta)
    }

    fn configure(meta: &mut ConstraintSystem<F>) -> Self::Config;
}
```

This allows runtime parametrization of the circuit configuration.  The extension to the Circuit trait has been designed to minimize the breaking change: existing circuits only need to define the associated `type Params`.

Unfortunately "Associated type defaults" are unstable in Rust, otherwise this would be a non-breaking change.  See rust-lang/rust#29661

* Implement circuit params under feature flag

* Don't overwrite configure method

* Fix doc test

* Allow halo2 constraint names to have non static names (privacy-scaling-explorations#156)

* static ref to String type in Gates, Constraints, VirtualCell, Argument

* 'lookup'.to_string()

* return &str for gate name and constriant_name, also run fmt

* Update halo2_gadgets/Cargo.toml

Co-authored-by: Han <[email protected]>

* upgrade rust-toochain

---------

Co-authored-by: Carlos Pérez <[email protected]>
Co-authored-by: Han <[email protected]>

* Improve halo2 query calls (privacy-scaling-explorations#154)

* return expression from cell

* add example

* selector

* recurse Expression to fill in index

* minimized changes from the original

* backword compatible meta.query_X & challange.expr()

* cargo fmt

* fixed lookup to pass all tests

* Update comments

Co-authored-by: Brecht Devos <[email protected]>

* Update comments

Co-authored-by: Brecht Devos <[email protected]>

* Update comments

Co-authored-by: Brecht Devos <[email protected]>

* Update comments

Co-authored-by: Brecht Devos <[email protected]>

* Update comments

Co-authored-by: Brecht Devos <[email protected]>

* Update comments

Co-authored-by: Brecht Devos <[email protected]>

* update

Co-authored-by: Brecht Devos <[email protected]>

* add primitives.rs back

* remove example2

* backward compatible meta.query_X & Column.cur(), next(), prev(), at(usize)

* impl Debug & make side effects only when query.index.is_none()

* change impl Debug for Expression instead & revert test in plonk_api

* upgrade rust-toolchain

* Update halo2_proofs/src/plonk/circuit.rs

Co-authored-by: Han <[email protected]>

* Update halo2_proofs/src/plonk/circuit.rs

Co-authored-by: Han <[email protected]>

* ran clippy

* Update halo2_proofs/src/plonk/circuit.rs

Co-authored-by: Han <[email protected]>

---------

Co-authored-by: Brecht Devos <[email protected]>
Co-authored-by: Han <[email protected]>

* fix: compute `num_chunks` more precisely (privacy-scaling-explorations#172)

* Implement Clone trait for Hash, Absorbing, and Sponge structs (privacy-scaling-explorations#171)

* Revert double-assignment mock prover check

Revert the check introduced in
privacy-scaling-explorations#129 to detect double
assignments with different values, because it breaks some tests in the zkevm
project.

There's a legitimate use case of double assignment with different values, which
is overwriting cells in order to perform negative tests (tests with bad witness
that should not pass the constraints).

Also in the EVM Circuit from the zkevm project we "abuse" the assignment of
cells as a cache: sometimes we assign some cells with a guess value, and later
on we reassign with the correct value.

I believe this check is interesting to have, so we could think of ways to add
it back as an optional feature.

* fix: Fix serialization for VerifyingKey (privacy-scaling-explorations#178)

Now the value returned when the number of selectors is a multiple of 8
is correct.

Resolves: privacy-scaling-explorations#175

* Add more getters to expose internal fields

* add a constructor (privacy-scaling-explorations#164)

* add a constructor

* add more comment

* fix as review

* remove clone

* remove

* no need to use new variable

* change comment

* fix clippy

* rename to from_parts

* remove n declaration

* feat: send sync region (privacy-scaling-explorations#180)

* feat: send / sync region

* Update layout.rs

* update

* lol

* debug

* Update keygen.rs

* Update keygen.rs

* Update keygen.rs

* Update keygen.rs

* thread-safe-region feature flag

* cleanup

* patch dev-graph

* patch non-determinism in mapping creation

* reduce mem usage for vk and pk

* mock proving examples

* swap for hashmap for insertion speed

* reduce update overhead

* replace BTree with Vec

* add benchmarks

* make the benchmarks massive

* patch clippy

* simplify lifetimes

* patch benches

* Update halo2_proofs/src/plonk/permutation/keygen.rs

Co-authored-by: Han <[email protected]>

* Update halo2_proofs/examples/vector-mul.rs

Co-authored-by: Han <[email protected]>

* rm benches

* order once

* patch lints

---------

Co-authored-by: Han <[email protected]>

* Fix `parallelize` workload imbalance (privacy-scaling-explorations#186)

* fix parallelize workload imbalance

* remove the need of unsafe

* implement native shuffle argument and api

* fix: remove nonsense comment

* strictly check shuffle rows

* address doc typos

* move compression into product commitment

* typo

* add shuffle errors for `verify_at_rows_par`

* dedup expression evaluation

* cargo fmt

* fix fields in sanity-checks feature

* Updates halo2_curves dependency to released package (privacy-scaling-explorations#190)

THe package release ressets the version from those inherited by the legacy
halo2curves repo's fork history.

The upstream diff is:
https://github.com/privacy-scaling-explorations/halo2curves/compare/9f5c50810bbefe779ee5cf1d852b2fe85dc35d5e..9a7f726fa74c8765bc7cdab11519cf285d169ecf

* chore: remove monorepo

Go back to having halo2curves and poseidon in separate repos.

* chore: fix clippy and tests

* fix: remove thread-safe-regions feature

`WitnessCollection` in `create_proof` isn't thread-safe.
We removed `Region`s from `SimpleLayouter` anyways.

* fix: rustfmt

* fix: dev-graph

* chore: update lint CI name

* chore: fix clippy

* chore: autoexample = false

turn off examples that use layouter

* chore(CI): separate job for examples

* chore: remove prefetch from asm, not used

* chore: fix asm feature

---------

Co-authored-by: adria0 <nowhere@>
Co-authored-by: Carlos Pérez <[email protected]>
Co-authored-by: adria0.eth <[email protected]>
Co-authored-by: Jonathan Wang <[email protected]>
Co-authored-by: kilic <[email protected]>
Co-authored-by: dante <[email protected]>
Co-authored-by: pinkiebell <[email protected]>
Co-authored-by: han0110 <[email protected]>
Co-authored-by: Eduard S <[email protected]>
Co-authored-by: naure <[email protected]>
Co-authored-by: Aurélien Nicolas <[email protected]>
Co-authored-by: CeciliaZ030 <[email protected]>
Co-authored-by: Brecht Devos <[email protected]>
Co-authored-by: Enrico Bottazzi <[email protected]>
Co-authored-by: Ethan-000 <[email protected]>
Co-authored-by: Mamy Ratsimbazafy <[email protected]>
Co-authored-by: kilic <[email protected]>
Co-authored-by: François Garillot <[email protected]>
pw-02 pushed a commit to pw-02/halo2 that referenced this pull request Sep 27, 2023
* feat: send / sync region

* Update layout.rs

* update

* lol

* debug

* Update keygen.rs

* Update keygen.rs

* Update keygen.rs

* Update keygen.rs

* thread-safe-region feature flag

* cleanup

* patch dev-graph

* patch non-determinism in mapping creation

* reduce mem usage for vk and pk

* mock proving examples

* swap for hashmap for insertion speed

* reduce update overhead

* replace BTree with Vec

* add benchmarks

* make the benchmarks massive

* patch clippy

* simplify lifetimes

* patch benches

* Update halo2_proofs/src/plonk/permutation/keygen.rs

Co-authored-by: Han <[email protected]>

* Update halo2_proofs/examples/vector-mul.rs

Co-authored-by: Han <[email protected]>

* rm benches

* order once

* patch lints

---------

Co-authored-by: Han <[email protected]>
Velaciela pushed a commit to scroll-tech/halo2 that referenced this pull request Oct 9, 2023
* feat: send / sync region

* Update layout.rs

* update

* lol

* debug

* Update keygen.rs

* Update keygen.rs

* Update keygen.rs

* Update keygen.rs

* thread-safe-region feature flag

* cleanup

* patch dev-graph

* patch non-determinism in mapping creation

* reduce mem usage for vk and pk

* mock proving examples

* swap for hashmap for insertion speed

* reduce update overhead

* replace BTree with Vec

* add benchmarks

* make the benchmarks massive

* patch clippy

* simplify lifetimes

* patch benches

* Update halo2_proofs/src/plonk/permutation/keygen.rs

Co-authored-by: Han <[email protected]>

* Update halo2_proofs/examples/vector-mul.rs

Co-authored-by: Han <[email protected]>

* rm benches

* order once

* patch lints

---------

Co-authored-by: Han <[email protected]>
kunxian-xia added a commit to scroll-tech/halo2 that referenced this pull request Jan 12, 2024
* feat: call synthesize in `MockProver` multiple times to behave same as real prover

* modify previous commit

* Expose mod `permutation` and re-export `permutation::keygen::Assembly` (privacy-scaling-explorations#149)

* feat: expose mod ule `permutation` and re-export `permutation::keygen::Assembly`

* feat: derive `lone` for `permutation::keygen::Assembly`

* feat: bump MSRV for `inferno`

* change: Migrate workspace to pasta_curves-0.5 (privacy-scaling-explorations#157)

* change: Migrate workspace to pasta_curves-0.5

This ports the majority of the workspace to the `pasta_curves-0.5.0`
leaving some tricky edge-cases that we need to handle carefully.

Resolves: privacy-scaling-explorations#132

* fix: Complete latest trait bounds to compile halo2proofs

* change: Migrate examples & benches to pasta 0.5

* change: Migrate halo2_gadgets to pasta-0.5

* change: Update gadgets outdated code with latest upstream

* fix: Sha3 gadget circuit

* fix: doc tests

* chore: Update merged main

* fix: Apply review suggestions

* fix previous commit

* Extend Circuit trait to take parameters in config (privacy-scaling-explorations#168)

* Extend Circuit trait to take parameters in config

The Circuit trait is extended with the following:
```
pub trait Circuit<F: Field> {
    /// [...]
    type Params: Default;

    fn params(&self) -> Self::Params {
        Self::Params::default()
    }

    fn configure_with_params(meta: &mut ConstraintSystem<F>, params: &Self::Params) -> Self::Config {
        Self::configure(meta)
    }

    fn configure(meta: &mut ConstraintSystem<F>) -> Self::Config;
}
```

This allows runtime parametrization of the circuit configuration.  The extension to the Circuit trait has been designed to minimize the breaking change: existing circuits only need to define the associated `type Params`.

Unfortunately "Associated type defaults" are unstable in Rust, otherwise this would be a non-breaking change.  See rust-lang/rust#29661

* Implement circuit params under feature flag

* Don't overwrite configure method

* Fix doc test

* Allow halo2 constraint names to have non static names (privacy-scaling-explorations#156)

* static ref to String type in Gates, Constraints, VirtualCell, Argument

* 'lookup'.to_string()

* return &str for gate name and constriant_name, also run fmt

* Update halo2_gadgets/Cargo.toml

Co-authored-by: Han <[email protected]>

* upgrade rust-toochain

---------

Co-authored-by: Carlos Pérez <[email protected]>
Co-authored-by: Han <[email protected]>

* Improve halo2 query calls (privacy-scaling-explorations#154)

* return expression from cell

* add example

* selector

* recurse Expression to fill in index

* minimized changes from the original

* backword compatible meta.query_X & challange.expr()

* cargo fmt

* fixed lookup to pass all tests

* Update comments

Co-authored-by: Brecht Devos <[email protected]>

* Update comments

Co-authored-by: Brecht Devos <[email protected]>

* Update comments

Co-authored-by: Brecht Devos <[email protected]>

* Update comments

Co-authored-by: Brecht Devos <[email protected]>

* Update comments

Co-authored-by: Brecht Devos <[email protected]>

* Update comments

Co-authored-by: Brecht Devos <[email protected]>

* update

Co-authored-by: Brecht Devos <[email protected]>

* add primitives.rs back

* remove example2

* backward compatible meta.query_X & Column.cur(), next(), prev(), at(usize)

* impl Debug & make side effects only when query.index.is_none()

* change impl Debug for Expression instead & revert test in plonk_api

* upgrade rust-toolchain

* Update halo2_proofs/src/plonk/circuit.rs

Co-authored-by: Han <[email protected]>

* Update halo2_proofs/src/plonk/circuit.rs

Co-authored-by: Han <[email protected]>

* ran clippy

* Update halo2_proofs/src/plonk/circuit.rs

Co-authored-by: Han <[email protected]>

---------

Co-authored-by: Brecht Devos <[email protected]>
Co-authored-by: Han <[email protected]>

* Implement Clone trait for Hash, Absorbing, and Sponge structs (privacy-scaling-explorations#171)

* fix: Fix serialization for VerifyingKey (privacy-scaling-explorations#178)

Now the value returned when the number of selectors is a multiple of 8
is correct.

Resolves: privacy-scaling-explorations#175

* Add more getters to expose internal fields

* add a constructor (privacy-scaling-explorations#164)

* add a constructor

* add more comment

* fix as review

* remove clone

* remove

* no need to use new variable

* change comment

* fix clippy

* rename to from_parts

* remove n declaration

* feat: send sync region (privacy-scaling-explorations#180)

* feat: send / sync region

* Update layout.rs

* update

* lol

* debug

* Update keygen.rs

* Update keygen.rs

* Update keygen.rs

* Update keygen.rs

* thread-safe-region feature flag

* cleanup

* patch dev-graph

* patch non-determinism in mapping creation

* reduce mem usage for vk and pk

* mock proving examples

* swap for hashmap for insertion speed

* reduce update overhead

* replace BTree with Vec

* add benchmarks

* make the benchmarks massive

* patch clippy

* simplify lifetimes

* patch benches

* Update halo2_proofs/src/plonk/permutation/keygen.rs

Co-authored-by: Han <[email protected]>

* Update halo2_proofs/examples/vector-mul.rs

Co-authored-by: Han <[email protected]>

* rm benches

* order once

* patch lints

---------

Co-authored-by: Han <[email protected]>

* fix previous commit

* Fix `parallelize` workload imbalance (privacy-scaling-explorations#186)

* fix parallelize workload imbalance

* remove the need of unsafe

* Updates halo2_curves dependency to released package (privacy-scaling-explorations#190)

THe package release ressets the version from those inherited by the legacy
halo2curves repo's fork history.

The upstream diff is:
https://github.com/privacy-scaling-explorations/halo2curves/compare/9f5c50810bbefe779ee5cf1d852b2fe85dc35d5e..9a7f726fa74c8765bc7cdab11519cf285d169ecf

* fix: explicitly define mds diff type (privacy-scaling-explorations#196)

* fix: explicitly define mds diff type

* rm paren

* feat: expose `transcript_repr` of `VerifyingKey` and reduce the trait constraint (privacy-scaling-explorations#200)

* implement native shuffle argument and api

fix: remove nonsense comment

strictly check shuffle rows

address doc typos

move compression into product commitment

typo

add shuffle errors for `verify_at_rows_par`

dedup expression evaluation

cargo fmt

fix fields in sanity-checks feature

* feat: public cells to allow for implementations of custom `Layouter`  (privacy-scaling-explorations#192)

* feat: public cells

* Update mds.rs

* Update mds.rs

* Update single_pass.rs

Co-authored-by: Han <[email protected]>

* bump toolchain to resolve errors

* fix clippy errors for CI run

* rustfmt post clippy

* plz let it be the last lint

* patch clippy lints in gadgets

* clippy lints for sha256 bench

* patch halo2proof benches

* Update assigned.rs

* Update halo2_gadgets/src/poseidon/primitives/mds.rs

Co-authored-by: Han <[email protected]>

* Update halo2_gadgets/src/poseidon/primitives/mds.rs

Co-authored-by: Han <[email protected]>

---------

Co-authored-by: Han <[email protected]>

* Synchronize with upstream (privacy-scaling-explorations#199)

* refactor: add default impl for `SyncDeps` for backward compatability

* feat: pick changes from zcash#728 and changes of flag `test-dev-graph`

* feat: pick changes from zcash#622

* feat: pick changes about mod `circuit` and mod `dev`

* feat: pick rest changes of `halo2_proofs`

* fix: when `--no-default-features`

* ci: sync from upstream, and deduplicate jobs when
push to `main`, and remove always failing job `codecov`.

* fix: make `commit_zk` runnable when `--no-default-features`

* chore: Update rust-toolchain to 1.66 for testing  (privacy-scaling-explorations#208)

* chore: Update rust-toolchain to 1.66 for testing

Note that tests will not compile due to the silent MSRV bump in
`blake2b_simd`.

Hence, we need to use `1.66` as toolchain.

Resolves: privacy-scaling-explorations#207

* change: UIpdate MSRVs in Cargo.toml

* fix: clippy (privacy-scaling-explorations#203)

* fix: clippy

* fmt

* fix: Final clippy complains & adjustments

---------

Co-authored-by: CPerezz <[email protected]>

* Implement Sum and Product for Expression (privacy-scaling-explorations#209)

* Make it Eq to make it easier for tests

* Implement Sum and Product for Expression

* Make it readable

* chore: update poseidon dependency

* fix: compiling bug with feautes=parallel_syn

* feat(MockProver): replace errors by asserts(privacy-scaling-explorations#150)

* boundary offset lost when resolving conflict

* disable multiphase prover

* Sync halo2 lib 0.4.0 merging (#81)

* Use thread pool for assign_regions (#57)

* feat: use rayon threadpool

* feat: add UT for many subregions

* refact: move common struct out to module level

* refact: reuse common configure code

* fix ci errors

---------

Co-authored-by: kunxian xia <[email protected]>

* Move `env_logger` dependency to dev-depdendencies (only for test). (#69)

* sync ff/group 0.13

* fix clippy

* fix clippy

* fmg

* [FEAT] Upgrading table16 for SHA256 (#73)

* upgrade sha256

* fix clippy

* Bus auto (#72)

* bus: expose global offset of regions

* bus-auto: add query_advice and query_fixed function in witness generation

* bus-auto: fix clippy

---------

Co-authored-by: Aurélien Nicolas <[email protected]>

* fix-tob-scroll-21 (#59)

* fix-tob-scroll-21

* expose param field for re-randomization

* enable accessing for table16 (#75)

* chore: update poseidon link

* merge sha256 gadget changes

* Fix the CI errors (#78)

* cargo fmt

* fix clippy error

* Feat: switch to logup scheme for lookup argument  (#71)

* Multi-input mv-lookup. (#49)

* Add mv_lookup.rs

* mv_lookup::prover, mv_lookup::verifier

* Replace lookup with mv_lookup

* replace halo2 with mv lookup

Co-authored-by: ying tong <[email protected]>

* cleanups

Co-authored-by: ying tong <[email protected]>

* ConstraintSystem: setup lookup_tracker

Co-authored-by: Andrija <[email protected]>

* mv_lookup::hybrid_prover

Co-authored-by: Andrija <[email protected]>

* WIP

* mv_multi_lookup: enable lookup caching

Co-authored-by: therealyingtong <[email protected]>

* Rename hybrid_lookup -> lookup

* Chunk lookups using user-provided minimum degree

Co-authored-by: Andrija <[email protected]>

* mv_lookup bench

Co-authored-by: Andrija <[email protected]>

* Introduce counter feature for FFTs and MSMs

Co-authored-by: Andrija <[email protected]>

* Fix off-by-one errors in chunk_lookup

Co-authored-by: Andrija <[email protected]>

* bench wip

* time evaluate_h

* KZG

* more efficient batch inversion

* extended lookup example

* Finalize mv lookup

Author: therealyingtong <[email protected]>

* Remove main/

* Fix according to the comments

* replace scan with parallel grand sum computation

* Revert Cargo.lock

* mv lookup Argument name

* parallel batch invert

---------

Co-authored-by: Andrija <[email protected]>
Co-authored-by: ying tong <[email protected]>
Co-authored-by: therealyingtong <[email protected]>

* fmt

* fix unit test

* fix clippy errors

* add todo in mv_lookup's prover

* fmt and clippy

* fix clippy

* add detailed running time of steps in logup's prover

* fmt

* add more log hooks

* more running time logs

* use par invert

* use sorted-vector to store how many times a table element occurs in input

* par the process to get inputs_inv_sum

* use par

* fix par

* add feature to skip inv sums

* add new feature flag

* fix clippy error

---------

Co-authored-by: Sphere L <[email protected]>
Co-authored-by: Andrija <[email protected]>
Co-authored-by: ying tong <[email protected]>
Co-authored-by: therealyingtong <[email protected]>

* fix some simple building errs

* upgrade pathfinder_simd to newer version as it can't compile on mac m1 pro

* resolve merge conflict

* fmt

* clippy

* more clippy fix

* more lint fix

* fmt

* minor syntax fix

* fix ipa multiopen test failure

* fix clippy warning

* fmt

* fix par scan of log_inv diff

* remove uncessary clone

---------

Co-authored-by: alannotnerd <[email protected]>
Co-authored-by: kunxian xia <[email protected]>
Co-authored-by: Steven <[email protected]>
Co-authored-by: Carlos Pérez <[email protected]>
Co-authored-by: zhenfei <[email protected]>
Co-authored-by: Ho <[email protected]>
Co-authored-by: naure <[email protected]>
Co-authored-by: Aurélien Nicolas <[email protected]>
Co-authored-by: Sphere L <[email protected]>
Co-authored-by: Andrija <[email protected]>
Co-authored-by: ying tong <[email protected]>
Co-authored-by: therealyingtong <[email protected]>

---------

Co-authored-by: han0110 <[email protected]>
Co-authored-by: Velaciela <[email protected]>
Co-authored-by: Carlos Pérez <[email protected]>
Co-authored-by: Eduard S <[email protected]>
Co-authored-by: CeciliaZ030 <[email protected]>
Co-authored-by: Brecht Devos <[email protected]>
Co-authored-by: Enrico Bottazzi <[email protected]>
Co-authored-by: Ethan-000 <[email protected]>
Co-authored-by: dante <[email protected]>
Co-authored-by: Mamy Ratsimbazafy <[email protected]>
Co-authored-by: François Garillot <[email protected]>
Co-authored-by: kilic <[email protected]>
Co-authored-by: Thor <[email protected]>
Co-authored-by: CPerezz <[email protected]>
Co-authored-by: chokermaxx <[email protected]>
Co-authored-by: Zhang Zhuo <[email protected]>
Co-authored-by: alannotnerd <[email protected]>
Co-authored-by: kunxian xia <[email protected]>
Co-authored-by: Steven <[email protected]>
Co-authored-by: Ho <[email protected]>
Co-authored-by: naure <[email protected]>
Co-authored-by: Aurélien Nicolas <[email protected]>
Co-authored-by: Sphere L <[email protected]>
Co-authored-by: Andrija <[email protected]>
Co-authored-by: ying tong <[email protected]>
Co-authored-by: therealyingtong <[email protected]>
iquerejeta pushed a commit to input-output-hk/halo2 that referenced this pull request May 8, 2024
…-hk/dev-feature/gl-166-cm-base-field-decomp

Add cost modeling for base-field scalar decomp

# Project Context

This corresponds to the general SOW goal of cost modeling.

The Galois internal issue is [Galois#166](https://gitlab-ext.galois.com/iog-midnight/halo2/-/issues/166).

# Issue Description

Back before the [Plonk gate gadget](privacy-scaling-explorations#178) existed, the cost-model tests for k-high-low decomp did not cover the `High-low-decomp correctness eq check` gate in ecc decomp. The `correctness eq check` gate and `high-low-decomp canonicity range check` gate were covered. Here we add tests to also cover the `High-low-decomp correctness eq check` gate. However, after rebasing onto the new Plonk gate gadget work, these custom gates don't exist anymore. But this still provides cost-model coverage of a gadget (fully-checke base-field scalar decomp) that was not cost-modeled before, this gadget is just implemented without internal gates now, using the Plonk gate gadget instead.

# Notes for Reviewers

In addition to the stated goal of adding cost-model tests for base-field scalar decomp, this PR also refactors some other, existing cost model tests.
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.

5 participants