From 03179e5674590dad2102854dd380e6bed035c850 Mon Sep 17 00:00:00 2001 From: Daniel Lubarov Date: Wed, 30 Jun 2021 12:54:45 -0700 Subject: [PATCH 01/21] Couple fixes related to blinding - `self.gates` -> `self.gate_instances` - Some tests were using a single binary FRI reduction, which doesn't provide enough succinctness for our blinding scheme to work. This caused `blinding_counts` to continue until it overflowed. --- src/bin/bench_recursion.rs | 2 +- src/circuit_builder.rs | 2 +- src/circuit_data.rs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bin/bench_recursion.rs b/src/bin/bench_recursion.rs index 59b65e51..0f1b9783 100644 --- a/src/bin/bench_recursion.rs +++ b/src/bin/bench_recursion.rs @@ -32,7 +32,7 @@ fn bench_prove, const D: usize>() { fri_config: FriConfig { proof_of_work_bits: 1, rate_bits: 3, - reduction_arity_bits: vec![1], + reduction_arity_bits: vec![1, 1, 1, 1], num_query_rounds: 1, }, }; diff --git a/src/circuit_builder.rs b/src/circuit_builder.rs index 1bf938a0..7ce4adb3 100644 --- a/src/circuit_builder.rs +++ b/src/circuit_builder.rs @@ -259,7 +259,7 @@ impl, const D: usize> CircuitBuilder { /// polynomials (which are opened at only one location) and for the Z polynomials (which are /// opened at two). fn blinding_counts(&self) -> (usize, usize) { - let num_gates = self.gates.len(); + let num_gates = self.gate_instances.len(); let mut degree_estimate = 1 << log2_ceil(num_gates); loop { diff --git a/src/circuit_data.rs b/src/circuit_data.rs index 6f352832..d72a0be3 100644 --- a/src/circuit_data.rs +++ b/src/circuit_data.rs @@ -39,7 +39,7 @@ impl Default for CircuitConfig { fri_config: FriConfig { proof_of_work_bits: 1, rate_bits: 1, - reduction_arity_bits: vec![1], + reduction_arity_bits: vec![1, 1, 1, 1], num_query_rounds: 1, }, } @@ -61,7 +61,7 @@ impl CircuitConfig { fri_config: FriConfig { proof_of_work_bits: 1, rate_bits: 3, - reduction_arity_bits: vec![1], + reduction_arity_bits: vec![1, 1, 1, 1], num_query_rounds: 1, }, } From 6c598ddcfd66658289a17e3d781568d36b997dc5 Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Wed, 30 Jun 2021 07:08:36 -0700 Subject: [PATCH 02/21] very initial attempt --- src/gadgets/insert.rs | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs index 64cf7299..8cb4614d 100644 --- a/src/gadgets/insert.rs +++ b/src/gadgets/insert.rs @@ -4,6 +4,16 @@ use crate::field::extension_field::Extendable; use crate::target::Target; impl, const D: usize> CircuitBuilder { + /// Evaluates to 1 if `x` and `y` are equal, 0 otherwise. + pub fn is_equal( + &mut self, + x: Target, + y: Target, + ) -> Target { + + } + + /// Inserts a `Target` in a vector at a non-deterministic index. This is done by rotating to the /// left, inserting at 0 and then rotating to the right. /// Note: `index` is not range-checked. @@ -13,9 +23,24 @@ impl, const D: usize> CircuitBuilder { element: ExtensionTarget, v: Vec>, ) -> Vec> { - let mut v = self.rotate_left(index, &v); - v.insert(0, element); - self.rotate_right(index, &v) + let mut already_inserted = self.zero(); + let mut new_list = Vec::new(); + + let mut cur_index = self.zero(); + for i in 0..v.len() { + cur_index = self.add(cur_index, self.one()); + let insert_here = self.is_equal(cur_index, index); + + let mut new_item = self.zero(); + new_item = self.add(new_item, self.mul(insert_here, element)); + new_item = self.add(new_item, self.mul(already_inserted, v[i-1])); + already_inserted = self.add(already_inserted, insert_here); + new_item = self.add(new_item, self.mul(self.sub(self.one(), already_inserted), v[i])); + + new_list.push(new_item); + } + + new_list } } #[cfg(test)] From af9a2c055c9844a8004603c22794cdbeeed5a3c8 Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Wed, 30 Jun 2021 14:02:20 -0700 Subject: [PATCH 03/21] some fixes --- src/gadgets/insert.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs index 8cb4614d..d1e0331d 100644 --- a/src/gadgets/insert.rs +++ b/src/gadgets/insert.rs @@ -35,7 +35,9 @@ impl, const D: usize> CircuitBuilder { new_item = self.add(new_item, self.mul(insert_here, element)); new_item = self.add(new_item, self.mul(already_inserted, v[i-1])); already_inserted = self.add(already_inserted, insert_here); - new_item = self.add(new_item, self.mul(self.sub(self.one(), already_inserted), v[i])); + + let not_already_inserted = self.sub(self.one(), already_inserted); + new_item = self.mul_add(not_already_inserted, v[i], new_item); new_list.push(new_item); } From 0c9d675eccc370df0df819c6637c6a73c6c05075 Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Wed, 30 Jun 2021 14:20:47 -0700 Subject: [PATCH 04/21] fixes --- src/field/extension_field/target.rs | 167 ++++++++++++++++++++++++++++ src/gadgets/insert.rs | 26 ++--- 2 files changed, 180 insertions(+), 13 deletions(-) diff --git a/src/field/extension_field/target.rs b/src/field/extension_field/target.rs index 9d60847e..ac881d8f 100644 --- a/src/field/extension_field/target.rs +++ b/src/field/extension_field/target.rs @@ -109,6 +109,173 @@ impl, const D: usize> CircuitBuilder { self.constant_ext_algebra(ExtensionAlgebra::ZERO) } + pub fn add_extension( + &mut self, + mut a: ExtensionTarget, + b: ExtensionTarget, + ) -> ExtensionTarget { + for i in 0..D { + a.0[i] = self.add(a.0[i], b.0[i]); + } + a + } + + pub fn add_ext_algebra( + &mut self, + mut a: ExtensionAlgebraTarget, + b: ExtensionAlgebraTarget, + ) -> ExtensionAlgebraTarget { + for i in 0..D { + a.0[i] = self.add_extension(a.0[i], b.0[i]); + } + a + } + + pub fn add_many_extension(&mut self, terms: &[ExtensionTarget]) -> ExtensionTarget { + let mut sum = self.zero_extension(); + for term in terms { + sum = self.add_extension(sum, *term); + } + sum + } + + /// TODO: Change this to using an `arithmetic_extension` function once `MulExtensionGate` supports addend. + pub fn sub_extension( + &mut self, + mut a: ExtensionTarget, + b: ExtensionTarget, + ) -> ExtensionTarget { + for i in 0..D { + a.0[i] = self.sub(a.0[i], b.0[i]); + } + a + } + + pub fn sub_ext_algebra( + &mut self, + mut a: ExtensionAlgebraTarget, + b: ExtensionAlgebraTarget, + ) -> ExtensionAlgebraTarget { + for i in 0..D { + a.0[i] = self.sub_extension(a.0[i], b.0[i]); + } + a + } + + pub fn mul_extension_with_const( + &mut self, + const_0: F, + multiplicand_0: ExtensionTarget, + multiplicand_1: ExtensionTarget, + ) -> ExtensionTarget { + let gate = self.add_gate(MulExtensionGate::new(), vec![const_0]); + + let wire_multiplicand_0 = + ExtensionTarget::from_range(gate, MulExtensionGate::::wires_multiplicand_0()); + let wire_multiplicand_1 = + ExtensionTarget::from_range(gate, MulExtensionGate::::wires_multiplicand_1()); + let wire_output = ExtensionTarget::from_range(gate, MulExtensionGate::::wires_output()); + + self.route_extension(multiplicand_0, wire_multiplicand_0); + self.route_extension(multiplicand_1, wire_multiplicand_1); + wire_output + } + + pub fn mul_extension( + &mut self, + multiplicand_0: ExtensionTarget, + multiplicand_1: ExtensionTarget, + ) -> ExtensionTarget { + self.mul_extension_with_const(F::ONE, multiplicand_0, multiplicand_1) + } + + pub fn mul_ext_algebra( + &mut self, + a: ExtensionAlgebraTarget, + b: ExtensionAlgebraTarget, + ) -> ExtensionAlgebraTarget { + let mut res = [self.zero_extension(); D]; + let w = self.constant(F::Extension::W); + for i in 0..D { + for j in 0..D { + let ai_bi = self.mul_extension(a.0[i], b.0[j]); + res[(i + j) % D] = if i + j < D { + self.add_extension(ai_bi, res[(i + j) % D]) + } else { + let w_ai_bi = self.scalar_mul_ext(w, ai_bi); + self.add_extension(w_ai_bi, res[(i + j) % D]) + } + } + } + ExtensionAlgebraTarget(res) + } + + pub fn mul_many_extension(&mut self, terms: &[ExtensionTarget]) -> ExtensionTarget { + let mut product = self.one_extension(); + for term in terms { + product = self.mul_extension(product, *term); + } + product + } + + /// Like `mul_add`, but for `ExtensionTarget`s. Note that, unlike `mul_add`, this has no + /// performance benefit over separate muls and adds. + /// TODO: Change this to using an `arithmetic_extension` function once `MulExtensionGate` supports addend. + pub fn mul_add_extension( + &mut self, + a: ExtensionTarget, + b: ExtensionTarget, + c: ExtensionTarget, + ) -> ExtensionTarget { + let product = self.mul_extension(a, b); + self.add_extension(product, c) + } + + /// Like `mul_add`, but for `ExtensionTarget`s. Note that, unlike `mul_add`, this has no + /// performance benefit over separate muls and subs. + /// TODO: Change this to using an `arithmetic_extension` function once `MulExtensionGate` supports addend. + pub fn scalar_mul_add_extension( + &mut self, + a: Target, + b: ExtensionTarget, + c: ExtensionTarget, + ) -> ExtensionTarget { + let product = self.scalar_mul_ext(a, b); + self.add_extension(product, c) + } + + /// Like `mul_sub`, but for `ExtensionTarget`s. Note that, unlike `mul_sub`, this has no + /// performance benefit over separate muls and subs. + /// TODO: Change this to using an `arithmetic_extension` function once `MulExtensionGate` supports addend. + pub fn scalar_mul_sub_extension( + &mut self, + a: Target, + b: ExtensionTarget, + c: ExtensionTarget, + ) -> ExtensionTarget { + let product = self.scalar_mul_ext(a, b); + self.sub_extension(product, c) + } + + /// Returns `a * b`, where `b` is in the extension field and `a` is in the base field. + pub fn scalar_mul_ext(&mut self, a: Target, b: ExtensionTarget) -> ExtensionTarget { + let a_ext = self.convert_to_ext(a); + self.mul_extension(a_ext, b) + } + + /// Returns `a * b`, where `b` is in the extension of the extension field, and `a` is in the + /// extension field. + pub fn scalar_mul_ext_algebra( + &mut self, + a: ExtensionTarget, + mut b: ExtensionAlgebraTarget, + ) -> ExtensionAlgebraTarget { + for i in 0..D { + b.0[i] = self.mul_extension(a, b.0[i]); + } + b + } + pub fn convert_to_ext(&mut self, t: Target) -> ExtensionTarget { let zero = self.zero(); let mut arr = [zero; D]; diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs index d1e0331d..aa310568 100644 --- a/src/gadgets/insert.rs +++ b/src/gadgets/insert.rs @@ -5,15 +5,10 @@ use crate::target::Target; impl, const D: usize> CircuitBuilder { /// Evaluates to 1 if `x` and `y` are equal, 0 otherwise. - pub fn is_equal( - &mut self, - x: Target, - y: Target, - ) -> Target { - + pub fn is_equal(&mut self, _x: Target, _y: Target) -> Target { + todo!() } - /// Inserts a `Target` in a vector at a non-deterministic index. This is done by rotating to the /// left, inserting at 0 and then rotating to the right. /// Note: `index` is not range-checked. @@ -28,16 +23,21 @@ impl, const D: usize> CircuitBuilder { let mut cur_index = self.zero(); for i in 0..v.len() { - cur_index = self.add(cur_index, self.one()); + let one = self.one(); + + cur_index = self.add(cur_index, one); let insert_here = self.is_equal(cur_index, index); - let mut new_item = self.zero(); - new_item = self.add(new_item, self.mul(insert_here, element)); - new_item = self.add(new_item, self.mul(already_inserted, v[i-1])); + let mut new_item = self.zero_extension(); + new_item = self.scalar_mul_add_extension(insert_here, element, new_item); + if i > 0 { + new_item = + self.scalar_mul_add_extension(already_inserted, v[i - 1], new_item); + } already_inserted = self.add(already_inserted, insert_here); - let not_already_inserted = self.sub(self.one(), already_inserted); - new_item = self.mul_add(not_already_inserted, v[i], new_item); + let not_already_inserted = self.sub(one, already_inserted); + new_item = self.scalar_mul_add_extension(not_already_inserted, v[i], new_item); new_list.push(new_item); } From 6cc06b408fab2b96d3faab7e9b2f082d692e1f92 Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Wed, 30 Jun 2021 14:23:05 -0700 Subject: [PATCH 05/21] small change --- src/gadgets/insert.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs index aa310568..b7f7e331 100644 --- a/src/gadgets/insert.rs +++ b/src/gadgets/insert.rs @@ -21,11 +21,10 @@ impl, const D: usize> CircuitBuilder { let mut already_inserted = self.zero(); let mut new_list = Vec::new(); - let mut cur_index = self.zero(); for i in 0..v.len() { let one = self.one(); - cur_index = self.add(cur_index, one); + let cur_index = self.constant(F::from_canonical_usize(i)); let insert_here = self.is_equal(cur_index, index); let mut new_item = self.zero_extension(); From 647568fc7ad0c35259ee9374c30e4db1836dde6f Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Wed, 30 Jun 2021 16:57:37 -0700 Subject: [PATCH 06/21] added EqualityGenerator --- src/generator.rs | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/generator.rs b/src/generator.rs index a2b35a53..130ca0d0 100644 --- a/src/generator.rs +++ b/src/generator.rs @@ -130,3 +130,38 @@ impl SimpleGenerator for RandomValueGenerator { PartialWitness::singleton_target(self.target, random_value) } } + + +/// A generator for including a random value +pub(crate) struct EqualityGenerator { + pub(crate) x: Target, + pub(crate) m: Target, + pub(crate) y: Target, +} + +impl SimpleGenerator for EqualityGenerator { + fn dependencies(&self) -> Vec { + vec![self.x] + } + + fn run_once(&self, witness: &PartialWitness) -> PartialWitness { + let x_value = witness.get_target(self.x); + + let y_value = if x_value == F::ZERO { + F::ZERO + } else { + F::ONE + }; + + let m_value = if x_value == F::ZERO { + F::ONE + } else { + x_value.inverse() + }; + + let mut witness = PartialWitness::new(); + witness.set_target(self.m, m_value); + witness.set_target(self.y, y_value); + witness + } +} From 77d942f0e9df99c0cbd4d5d22f80c6f2ce5a356d Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Wed, 30 Jun 2021 16:58:06 -0700 Subject: [PATCH 07/21] cleanup --- src/circuit_builder.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/circuit_builder.rs b/src/circuit_builder.rs index 7ce4adb3..d9ff162e 100644 --- a/src/circuit_builder.rs +++ b/src/circuit_builder.rs @@ -310,16 +310,16 @@ impl, const D: usize> CircuitBuilder { input: w, }), }); - self.add_generator(CopyGenerator { - src: Target::Wire(Wire { + self.generate_copy( + Target::Wire(Wire { gate: gate_1, input: w, }), - dst: Target::Wire(Wire { + Target::Wire(Wire { gate: gate_2, input: w, }), - }); + ); } } From 80758da0f4cbe62ddb2ad459601bde15d2a9e1fd Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Wed, 30 Jun 2021 16:58:13 -0700 Subject: [PATCH 08/21] removed duplicate functions --- src/field/extension_field/target.rs | 167 ---------------------------- 1 file changed, 167 deletions(-) diff --git a/src/field/extension_field/target.rs b/src/field/extension_field/target.rs index ac881d8f..9d60847e 100644 --- a/src/field/extension_field/target.rs +++ b/src/field/extension_field/target.rs @@ -109,173 +109,6 @@ impl, const D: usize> CircuitBuilder { self.constant_ext_algebra(ExtensionAlgebra::ZERO) } - pub fn add_extension( - &mut self, - mut a: ExtensionTarget, - b: ExtensionTarget, - ) -> ExtensionTarget { - for i in 0..D { - a.0[i] = self.add(a.0[i], b.0[i]); - } - a - } - - pub fn add_ext_algebra( - &mut self, - mut a: ExtensionAlgebraTarget, - b: ExtensionAlgebraTarget, - ) -> ExtensionAlgebraTarget { - for i in 0..D { - a.0[i] = self.add_extension(a.0[i], b.0[i]); - } - a - } - - pub fn add_many_extension(&mut self, terms: &[ExtensionTarget]) -> ExtensionTarget { - let mut sum = self.zero_extension(); - for term in terms { - sum = self.add_extension(sum, *term); - } - sum - } - - /// TODO: Change this to using an `arithmetic_extension` function once `MulExtensionGate` supports addend. - pub fn sub_extension( - &mut self, - mut a: ExtensionTarget, - b: ExtensionTarget, - ) -> ExtensionTarget { - for i in 0..D { - a.0[i] = self.sub(a.0[i], b.0[i]); - } - a - } - - pub fn sub_ext_algebra( - &mut self, - mut a: ExtensionAlgebraTarget, - b: ExtensionAlgebraTarget, - ) -> ExtensionAlgebraTarget { - for i in 0..D { - a.0[i] = self.sub_extension(a.0[i], b.0[i]); - } - a - } - - pub fn mul_extension_with_const( - &mut self, - const_0: F, - multiplicand_0: ExtensionTarget, - multiplicand_1: ExtensionTarget, - ) -> ExtensionTarget { - let gate = self.add_gate(MulExtensionGate::new(), vec![const_0]); - - let wire_multiplicand_0 = - ExtensionTarget::from_range(gate, MulExtensionGate::::wires_multiplicand_0()); - let wire_multiplicand_1 = - ExtensionTarget::from_range(gate, MulExtensionGate::::wires_multiplicand_1()); - let wire_output = ExtensionTarget::from_range(gate, MulExtensionGate::::wires_output()); - - self.route_extension(multiplicand_0, wire_multiplicand_0); - self.route_extension(multiplicand_1, wire_multiplicand_1); - wire_output - } - - pub fn mul_extension( - &mut self, - multiplicand_0: ExtensionTarget, - multiplicand_1: ExtensionTarget, - ) -> ExtensionTarget { - self.mul_extension_with_const(F::ONE, multiplicand_0, multiplicand_1) - } - - pub fn mul_ext_algebra( - &mut self, - a: ExtensionAlgebraTarget, - b: ExtensionAlgebraTarget, - ) -> ExtensionAlgebraTarget { - let mut res = [self.zero_extension(); D]; - let w = self.constant(F::Extension::W); - for i in 0..D { - for j in 0..D { - let ai_bi = self.mul_extension(a.0[i], b.0[j]); - res[(i + j) % D] = if i + j < D { - self.add_extension(ai_bi, res[(i + j) % D]) - } else { - let w_ai_bi = self.scalar_mul_ext(w, ai_bi); - self.add_extension(w_ai_bi, res[(i + j) % D]) - } - } - } - ExtensionAlgebraTarget(res) - } - - pub fn mul_many_extension(&mut self, terms: &[ExtensionTarget]) -> ExtensionTarget { - let mut product = self.one_extension(); - for term in terms { - product = self.mul_extension(product, *term); - } - product - } - - /// Like `mul_add`, but for `ExtensionTarget`s. Note that, unlike `mul_add`, this has no - /// performance benefit over separate muls and adds. - /// TODO: Change this to using an `arithmetic_extension` function once `MulExtensionGate` supports addend. - pub fn mul_add_extension( - &mut self, - a: ExtensionTarget, - b: ExtensionTarget, - c: ExtensionTarget, - ) -> ExtensionTarget { - let product = self.mul_extension(a, b); - self.add_extension(product, c) - } - - /// Like `mul_add`, but for `ExtensionTarget`s. Note that, unlike `mul_add`, this has no - /// performance benefit over separate muls and subs. - /// TODO: Change this to using an `arithmetic_extension` function once `MulExtensionGate` supports addend. - pub fn scalar_mul_add_extension( - &mut self, - a: Target, - b: ExtensionTarget, - c: ExtensionTarget, - ) -> ExtensionTarget { - let product = self.scalar_mul_ext(a, b); - self.add_extension(product, c) - } - - /// Like `mul_sub`, but for `ExtensionTarget`s. Note that, unlike `mul_sub`, this has no - /// performance benefit over separate muls and subs. - /// TODO: Change this to using an `arithmetic_extension` function once `MulExtensionGate` supports addend. - pub fn scalar_mul_sub_extension( - &mut self, - a: Target, - b: ExtensionTarget, - c: ExtensionTarget, - ) -> ExtensionTarget { - let product = self.scalar_mul_ext(a, b); - self.sub_extension(product, c) - } - - /// Returns `a * b`, where `b` is in the extension field and `a` is in the base field. - pub fn scalar_mul_ext(&mut self, a: Target, b: ExtensionTarget) -> ExtensionTarget { - let a_ext = self.convert_to_ext(a); - self.mul_extension(a_ext, b) - } - - /// Returns `a * b`, where `b` is in the extension of the extension field, and `a` is in the - /// extension field. - pub fn scalar_mul_ext_algebra( - &mut self, - a: ExtensionTarget, - mut b: ExtensionAlgebraTarget, - ) -> ExtensionAlgebraTarget { - for i in 0..D { - b.0[i] = self.mul_extension(a, b.0[i]); - } - b - } - pub fn convert_to_ext(&mut self, t: Target) -> ExtensionTarget { let zero = self.zero(); let mut arr = [zero; D]; From cad7dc6904b05d53e9e9ce72aea47fb411d8b3aa Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Wed, 30 Jun 2021 21:31:17 -0700 Subject: [PATCH 09/21] some progress --- src/gadgets/insert.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs index b7f7e331..4b6a15b1 100644 --- a/src/gadgets/insert.rs +++ b/src/gadgets/insert.rs @@ -2,11 +2,26 @@ use crate::circuit_builder::CircuitBuilder; use crate::field::extension_field::target::ExtensionTarget; use crate::field::extension_field::Extendable; use crate::target::Target; +use crate::generator::EqualityGenerator; impl, const D: usize> CircuitBuilder { + /// Evaluates to 1 if `x` equals zero, 0 otherwise. + pub fn is_zero(&mut self, x: Target) -> Target { + let m = todo!(); + let y = todo!(); + + self.add_generator(EqualityGenerator { + x, + m, + y, + }); + + y + } + /// Evaluates to 1 if `x` and `y` are equal, 0 otherwise. - pub fn is_equal(&mut self, _x: Target, _y: Target) -> Target { - todo!() + pub fn is_equal(&mut self, x: Target, y: Target) -> Target { + self.is_zero(self.sub(x, y)) } /// Inserts a `Target` in a vector at a non-deterministic index. This is done by rotating to the From 574a3d4847fc3ca8506849c611bde55e639f9adf Mon Sep 17 00:00:00 2001 From: Hamish Ivey-Law <426294+unzvfu@users.noreply.github.com> Date: Thu, 1 Jul 2021 14:55:41 +1000 Subject: [PATCH 10/21] FFT improvements (#81) * Use built-in `reverse_bits`; remove duplicate `reverse_index_bits`. * Reduce precomputation time/space complexity from quadratic to linear. * Several working cache-friendly FFTs. * Fix to allow FFT of constant polynomial. * Simplify FFT strategy choice. * Add PrimeField and CHARACTERISTIC properties to Fields. * Add faster method for inverse of 2^m. * Pre-compute some of the roots; tidy up loop iteration. * Precomputation for both FFT variants. * Refactor precomputation; add optional parameters; rename some things. * Unrolled version with zero tail. * Iterative version of Unrolled precomputation. * Test zero tail algo. * Restore default degree. * Address comments from @dlubarov and @wborgeaud. --- src/field/crandall_field.rs | 3 + src/field/extension_field/quadratic.rs | 3 + src/field/extension_field/quartic.rs | 3 + src/field/fft.rs | 358 ++++++++++++++++++------- src/field/field.rs | 28 ++ src/field/field_testing.rs | 14 + src/polynomial/polynomial.rs | 6 +- src/util/mod.rs | 15 +- 8 files changed, 325 insertions(+), 105 deletions(-) diff --git a/src/field/crandall_field.rs b/src/field/crandall_field.rs index dbd29cb2..7a1d18d6 100644 --- a/src/field/crandall_field.rs +++ b/src/field/crandall_field.rs @@ -136,6 +136,8 @@ impl Debug for CrandallField { } impl Field for CrandallField { + type PrimeField = Self; + const ZERO: Self = Self(0); const ONE: Self = Self(1); const TWO: Self = Self(2); @@ -143,6 +145,7 @@ impl Field for CrandallField { const ORDER: u64 = 18446744071293632513; const TWO_ADICITY: usize = 28; + const CHARACTERISTIC: u64 = Self::ORDER; const MULTIPLICATIVE_GROUP_GENERATOR: Self = Self(5); const POWER_OF_TWO_GENERATOR: Self = Self(10281950781551402419); diff --git a/src/field/extension_field/quadratic.rs b/src/field/extension_field/quadratic.rs index af21ad60..ede2ef26 100644 --- a/src/field/extension_field/quadratic.rs +++ b/src/field/extension_field/quadratic.rs @@ -43,11 +43,14 @@ impl From<>::BaseField> for QuadraticCrandallField { } impl Field for QuadraticCrandallField { + type PrimeField = CrandallField; + const ZERO: Self = Self([CrandallField::ZERO; 2]); const ONE: Self = Self([CrandallField::ONE, CrandallField::ZERO]); const TWO: Self = Self([CrandallField::TWO, CrandallField::ZERO]); const NEG_ONE: Self = Self([CrandallField::NEG_ONE, CrandallField::ZERO]); + const CHARACTERISTIC: u64 = CrandallField::ORDER; // Does not fit in 64-bits. const ORDER: u64 = 0; const TWO_ADICITY: usize = 29; diff --git a/src/field/extension_field/quartic.rs b/src/field/extension_field/quartic.rs index b93cbb56..f609eeb7 100644 --- a/src/field/extension_field/quartic.rs +++ b/src/field/extension_field/quartic.rs @@ -50,6 +50,8 @@ impl From<>::BaseField> for QuarticCrandallField { } impl Field for QuarticCrandallField { + type PrimeField = CrandallField; + const ZERO: Self = Self([CrandallField::ZERO; 4]); const ONE: Self = Self([ CrandallField::ONE, @@ -70,6 +72,7 @@ impl Field for QuarticCrandallField { CrandallField::ZERO, ]); + const CHARACTERISTIC: u64 = CrandallField::ORDER; // Does not fit in 64-bits. const ORDER: u64 = 0; const TWO_ADICITY: usize = 30; diff --git a/src/field/fft.rs b/src/field/fft.rs index 56764b47..af5c05a7 100644 --- a/src/field/fft.rs +++ b/src/field/fft.rs @@ -1,142 +1,304 @@ +use std::option::Option; + use crate::field::field::Field; use crate::polynomial::polynomial::{PolynomialCoeffs, PolynomialValues}; -use crate::util::{log2_ceil, log2_strict}; +use crate::util::{log2_strict, reverse_index_bits}; -/// Permutes `arr` such that each index is mapped to its reverse in binary. -fn reverse_index_bits(arr: Vec) -> Vec { - let n = arr.len(); - let n_power = log2_strict(n); +// TODO: Should really do some "dynamic" dispatch to handle the +// different FFT algos rather than C-style enum dispatch. +enum FftStrategy { Classic, Unrolled } - let mut result = Vec::with_capacity(n); - for i in 0..n { - result.push(arr[reverse_bits(i, n_power)]); +const FFT_STRATEGY: FftStrategy = FftStrategy::Classic; + +type FftRootTable = Vec>; + +fn fft_classic_root_table(n: usize) -> FftRootTable { + let lg_n = log2_strict(n); + // bases[i] = g^2^i, for i = 0, ..., lg_n - 1 + let mut bases = Vec::with_capacity(lg_n); + let mut base = F::primitive_root_of_unity(lg_n); + bases.push(base); + for _ in 1..lg_n { + base = base.square(); // base = g^2^_ + bases.push(base); } - result -} -fn reverse_bits(n: usize, num_bits: usize) -> usize { - let mut result = 0; - for i in 0..num_bits { - let i_rev = num_bits - i - 1; - result |= (n >> i & 1) << i_rev; + let mut root_table = Vec::with_capacity(lg_n); + for lg_m in 1..=lg_n { + let half_m = 1 << (lg_m - 1); + let base = bases[lg_n - lg_m]; + let root_row = base.powers().take(half_m.max(2)).collect(); + root_table.push(root_row); } - result + root_table } -pub(crate) struct FftPrecomputation { - /// For each layer index i, stores the cyclic subgroup corresponding to the evaluation domain of - /// layer i. The indices within these subgroup vectors are bit-reversed. - subgroups_rev: Vec>, + +fn fft_unrolled_root_table(n: usize) -> FftRootTable { + // Precompute a table of the roots of unity used in the main + // loops. + + // Suppose n is the size of the outer vector and g is a primitive nth + // root of unity. Then the [lg(m) - 1][j] element of the table is + // g^{ n/2m * j } for j = 0..m-1 + + let lg_n = log2_strict(n); + // bases[i] = g^2^i, for i = 0, ..., lg_n - 2 + let mut bases = Vec::with_capacity(lg_n); + let mut base = F::primitive_root_of_unity(lg_n); + bases.push(base); + // NB: If n = 1, then lg_n is zero, so we can't do 1..(lg_n-1) here + for _ in 2..lg_n { + base = base.square(); // base = g^2^(_-1) + bases.push(base); + } + + let mut root_table = Vec::with_capacity(lg_n); + for lg_m in 1..lg_n { + let m = 1 << lg_m; + let base = bases[lg_n - lg_m - 1]; + let root_row = base.powers().take(m.max(2)).collect(); + root_table.push(root_row); + } + root_table } -impl FftPrecomputation { - pub fn size(&self) -> usize { - self.subgroups_rev.last().unwrap().len() +#[inline] +fn fft_dispatch( + input: Vec, + zero_factor: Option, + root_table: Option> +) -> Vec { + let n = input.len(); + match FFT_STRATEGY { + FftStrategy::Classic + => fft_classic(input, + zero_factor.unwrap_or(0), + root_table.unwrap_or_else(|| fft_classic_root_table(n))), + FftStrategy::Unrolled + => fft_unrolled(input, + zero_factor.unwrap_or(0), + root_table.unwrap_or_else(|| fft_unrolled_root_table(n))) } } +#[inline] pub fn fft(poly: PolynomialCoeffs) -> PolynomialValues { - let precomputation = fft_precompute(poly.len()); - fft_with_precomputation_power_of_2(poly, &precomputation) + fft_with_options(poly, None, None) } -pub(crate) fn fft_precompute(degree: usize) -> FftPrecomputation { - let degree_log = log2_ceil(degree); - - let mut subgroups_rev = Vec::new(); - let mut subgroup = F::two_adic_subgroup(degree_log); - for _i in 0..=degree_log { - let subsubgroup = subgroup.iter().step_by(2).copied().collect(); - let subgroup_rev = reverse_index_bits(subgroup); - subgroups_rev.push(subgroup_rev); - subgroup = subsubgroup; - } - subgroups_rev.reverse(); - - FftPrecomputation { subgroups_rev } +#[inline] +pub fn fft_with_options( + poly: PolynomialCoeffs, + zero_factor: Option, + root_table: Option> +) -> PolynomialValues { + let PolynomialCoeffs { coeffs } = poly; + PolynomialValues { values: fft_dispatch(coeffs, zero_factor, root_table) } } -pub(crate) fn ifft_with_precomputation_power_of_2( +#[inline] +pub fn ifft(poly: PolynomialValues) -> PolynomialCoeffs { + ifft_with_options(poly, None, None) +} + +pub fn ifft_with_options( poly: PolynomialValues, - precomputation: &FftPrecomputation, + zero_factor: Option, + root_table: Option> ) -> PolynomialCoeffs { let n = poly.len(); - let n_inv = F::from_canonical_usize(n).try_inverse().unwrap(); + let lg_n = log2_strict(n); + let n_inv = F::inverse_2exp(lg_n); let PolynomialValues { values } = poly; - let PolynomialValues { values: mut result } = - fft_with_precomputation_power_of_2(PolynomialCoeffs { coeffs: values }, precomputation); + let mut coeffs = fft_dispatch(values, zero_factor, root_table); // We reverse all values except the first, and divide each by n. - result[0] *= n_inv; - result[n / 2] *= n_inv; + coeffs[0] *= n_inv; + coeffs[n / 2] *= n_inv; for i in 1..(n / 2) { let j = n - i; - let result_i = result[j] * n_inv; - let result_j = result[i] * n_inv; - result[i] = result_i; - result[j] = result_j; + let coeffs_i = coeffs[j] * n_inv; + let coeffs_j = coeffs[i] * n_inv; + coeffs[i] = coeffs_i; + coeffs[j] = coeffs_j; } - PolynomialCoeffs { coeffs: result } + PolynomialCoeffs { coeffs } } -pub(crate) fn fft_with_precomputation_power_of_2( - poly: PolynomialCoeffs, - precomputation: &FftPrecomputation, -) -> PolynomialValues { - debug_assert_eq!( - poly.len(), - precomputation.subgroups_rev.last().unwrap().len(), - "Number of coefficients does not match size of subgroup in precomputation" - ); +/// FFT implementation based on Section 32.3 of "Introduction to +/// Algorithms" by Cormen et al. +/// +/// The parameter r signifies that the first 1/2^r of the entries of +/// input may be non-zero, but the last 1 - 1/2^r entries are +/// definitely zero. +pub(crate) fn fft_classic( + input: Vec, + r: usize, + root_table: FftRootTable +) -> Vec { + let mut values = reverse_index_bits(input); - let half_degree = poly.len() >> 1; - let degree_log = poly.log_len(); + let n = values.len(); + let lg_n = log2_strict(n); - // In the base layer, we're just evaluating "degree 0 polynomials", i.e. the coefficients - // themselves. - let PolynomialCoeffs { coeffs } = poly; - let mut evaluations = reverse_index_bits(coeffs); + if root_table.len() != lg_n { + panic!("Expected root table of length {}, but it was {}.", lg_n, root_table.len()); + } - for i in 1..=degree_log { - // In layer i, we're evaluating a series of polynomials, each at 2^i points. In practice - // we evaluate a pair of points together, so we have 2^(i - 1) pairs. - let points_per_poly = 1 << i; - let pairs_per_poly = 1 << (i - 1); - - let mut new_evaluations = Vec::new(); - for pair_index in 0..half_degree { - let poly_index = pair_index / pairs_per_poly; - let pair_index_within_poly = pair_index % pairs_per_poly; - - let child_index_0 = poly_index * points_per_poly + pair_index_within_poly; - let child_index_1 = child_index_0 + pairs_per_poly; - - let even = evaluations[child_index_0]; - let odd = evaluations[child_index_1]; - - let point_0 = precomputation.subgroups_rev[i][pair_index_within_poly * 2]; - let product = point_0 * odd; - new_evaluations.push(even + product); - new_evaluations.push(even - product); + // After reverse_index_bits, the only non-zero elements of values + // are at indices i*2^r for i = 0..n/2^r. The loop below copies + // the value at i*2^r to the positions [i*2^r + 1, i*2^r + 2, ..., + // (i+1)*2^r - 1]; i.e. it replaces the 2^r - 1 zeros following + // element i*2^r with the value at i*2^r. This corresponds to the + // first r rounds of the FFT when there are 2^r zeros at the end + // of the original input. + if r > 0 { // if r == 0 then this loop is a noop. + let mask = !((1 << r) - 1); + for i in 0..n { + values[i] = values[i & mask]; } - evaluations = new_evaluations; } - // Reorder so that evaluations' indices correspond to (g_0, g_1, g_2, ...) - let values = reverse_index_bits(evaluations); - PolynomialValues { values } + let mut m = 1 << (r + 1); + for lg_m in (r+1)..=lg_n { + let half_m = m / 2; + for k in (0..n).step_by(m) { + for j in 0..half_m { + let omega = root_table[lg_m - 1][j]; + let t = omega * values[k + half_m + j]; + let u = values[k + j]; + values[k + j] = u + t; + values[k + half_m + j] = u - t; + } + } + m *= 2; + } + values } -pub(crate) fn ifft(poly: PolynomialValues) -> PolynomialCoeffs { - let precomputation = fft_precompute(poly.len()); - ifft_with_precomputation_power_of_2(poly, &precomputation) +/// FFT implementation inspired by Barretenberg's (but with extra unrolling): +/// https://github.com/AztecProtocol/barretenberg/blob/master/barretenberg/src/aztec/polynomials/polynomial_arithmetic.cpp#L58 +/// https://github.com/AztecProtocol/barretenberg/blob/master/barretenberg/src/aztec/polynomials/evaluation_domain.cpp#L30 +/// +/// The parameter r signifies that the first 1/2^r of the entries of +/// input may be non-zero, but the last 1 - 1/2^r entries are +/// definitely zero. +fn fft_unrolled( + input: Vec, + r_orig: usize, + root_table: FftRootTable +) -> Vec { + let n = input.len(); + let lg_n = log2_strict(input.len()); + + let mut values = reverse_index_bits(input); + + // FFT of a constant polynomial (including zero) is itself. + if n < 2 { + return values + } + + // The 'm' corresponds to the specialisation from the 'm' in the + // main loop (m >= 4) below. + + // (See comment in fft_classic near same code.) + let mut r = r_orig; + let mut m = 1 << r; + if r > 0 { // if r == 0 then this loop is a noop. + let mask = !((1 << r) - 1); + for i in 0..n { + values[i] = values[i & mask]; + } + } + + // m = 1 + if m == 1 { + for k in (0..n).step_by(2) { + let t = values[k + 1]; + values[k + 1] = values[k] - t; + values[k] += t; + } + r += 1; + m *= 2; + } + + if n == 2 { + return values + } + + if root_table.len() != (lg_n - 1) { + panic!("Expected root table of length {}, but it was {}.", lg_n, root_table.len()); + } + + // m = 2 + if m <= 2 { + for k in (0..n).step_by(4) { + // NB: Grouping statements as is done in the main loop below + // does not seem to help here (worse by a few millis). + let omega_0 = root_table[0][0]; + let tmp_0 = omega_0 * values[k + 2 + 0]; + values[k + 2 + 0] = values[k + 0] - tmp_0; + values[k + 0] += tmp_0; + + let omega_1 = root_table[0][1]; + let tmp_1 = omega_1 * values[k + 2 + 1]; + values[k + 2 + 1] = values[k + 1] - tmp_1; + values[k + 1] += tmp_1; + } + r += 1; + m *= 2; + } + + // m >= 4 + for lg_m in r..lg_n { + for k in (0..n).step_by(2*m) { + // Unrolled the commented loop by groups of 4 and + // rearranged the lines. Improves runtime by about + // 10%. + /* + for j in (0..m) { + let omega = root_table[lg_m - 1][j]; + let tmp = omega * values[k + m + j]; + values[k + m + j] = values[k + j] - tmp; + values[k + j] += tmp; + } + */ + for j in (0..m).step_by(4) { + let off1 = k + j; + let off2 = k + m + j; + + let omega_0 = root_table[lg_m - 1][j + 0]; + let omega_1 = root_table[lg_m - 1][j + 1]; + let omega_2 = root_table[lg_m - 1][j + 2]; + let omega_3 = root_table[lg_m - 1][j + 3]; + + let tmp_0 = omega_0 * values[off2 + 0]; + let tmp_1 = omega_1 * values[off2 + 1]; + let tmp_2 = omega_2 * values[off2 + 2]; + let tmp_3 = omega_3 * values[off2 + 3]; + + values[off2 + 0] = values[off1 + 0] - tmp_0; + values[off2 + 1] = values[off1 + 1] - tmp_1; + values[off2 + 2] = values[off1 + 2] - tmp_2; + values[off2 + 3] = values[off1 + 3] - tmp_3; + values[off1 + 0] += tmp_0; + values[off1 + 1] += tmp_1; + values[off1 + 2] += tmp_2; + values[off1 + 3] += tmp_3; + } + } + m *= 2; + } + values } + #[cfg(test)] mod tests { use crate::field::crandall_field::CrandallField; - use crate::field::fft::{fft, ifft}; + use crate::field::fft::{fft, ifft, fft_with_options}; use crate::field::field::Field; use crate::polynomial::polynomial::{PolynomialCoeffs, PolynomialValues}; use crate::util::{log2_ceil, log2_strict}; @@ -162,6 +324,12 @@ mod tests { for i in degree..degree_padded { assert_eq!(interpolated_coefficients.coeffs[i], F::ZERO); } + + for r in 0..4 { + // expand ceofficients by factor 2^r by filling with zeros + let zero_tail = coefficients.clone().lde(r); + assert_eq!(fft(zero_tail.clone()), fft_with_options(zero_tail, Some(r), None)); + } } fn evaluate_naive(coefficients: &PolynomialCoeffs) -> PolynomialValues { diff --git a/src/field/field.rs b/src/field/field.rs index 516012d2..f4ef5990 100644 --- a/src/field/field.rs +++ b/src/field/field.rs @@ -32,11 +32,14 @@ pub trait Field: + Send + Sync { + type PrimeField: Field; + const ZERO: Self; const ONE: Self; const TWO: Self; const NEG_ONE: Self; + const CHARACTERISTIC: u64; const ORDER: u64; const TWO_ADICITY: usize; @@ -101,6 +104,31 @@ pub trait Field: x_inv } + /// Compute the inverse of 2^exp in this field. + #[inline] + fn inverse_2exp(exp: usize) -> Self { + let p = Self::CHARACTERISTIC; + + if exp <= Self::PrimeField::TWO_ADICITY { + // The inverse of 2^exp is p-(p-1)/2^exp when char(F) = p and exp is + // at most the TWO_ADICITY of the prime field. + // + // NB: PrimeFields fit in 64 bits => TWO_ADICITY < 64 => + // exp < 64 => this shift amount is legal. + Self::from_canonical_u64(p - ((p - 1) >> exp)) + } else { + // In the general case we compute 1/2 = (p+1)/2 and then exponentiate + // by exp to get 1/2^exp. Costs about log_2(exp) operations. + let half = Self::from_canonical_u64((p + 1) >> 1); + half.exp(exp as u64) + + // TODO: Faster to combine several high powers of 1/2 using multiple + // applications of the trick above. E.g. if the 2-adicity is v, then + // compute 1/2^(v^2 + v + 13) with 1/2^((v + 1) * v + 13), etc. + // (using the v-adic expansion of m). Costs about log_v(exp) operations. + } + } + fn primitive_root_of_unity(n_log: usize) -> Self { assert!(n_log <= Self::TWO_ADICITY); let mut base = Self::POWER_OF_TWO_GENERATOR; diff --git a/src/field/field_testing.rs b/src/field/field_testing.rs index 53e9c63c..7190684f 100644 --- a/src/field/field_testing.rs +++ b/src/field/field_testing.rs @@ -315,6 +315,20 @@ macro_rules! test_arithmetic { assert_eq!(x, F::ONE); assert_eq!(F::ZERO - x, F::NEG_ONE); } + + #[test] + fn inverse_2exp() { + // Just check consistency with try_inverse() + type F = $field; + + let v = ::PrimeField::TWO_ADICITY; + + for e in [0, 1, 2, 3, 4, v - 2, v - 1, v, v + 1, v + 2, 123*v] { + let x = F::TWO.exp(e as u64).inverse(); + let y = F::inverse_2exp(e); + assert_eq!(x, y); + } + } } }; } diff --git a/src/polynomial/polynomial.rs b/src/polynomial/polynomial.rs index 888d7af0..5f295030 100644 --- a/src/polynomial/polynomial.rs +++ b/src/polynomial/polynomial.rs @@ -1,3 +1,5 @@ +use std::time::Instant; + use std::cmp::max; use std::iter::Sum; use std::ops::{Add, AddAssign, Mul, MulAssign, Sub, SubAssign}; @@ -5,7 +7,7 @@ use std::ops::{Add, AddAssign, Mul, MulAssign, Sub, SubAssign}; use anyhow::{ensure, Result}; use crate::field::extension_field::Extendable; -use crate::field::fft::{fft, ifft}; +use crate::field::fft::{fft, ifft, fft_with_options}; use crate::field::field::Field; use crate::util::log2_strict; @@ -55,7 +57,7 @@ impl PolynomialValues { pub fn lde(self, rate_bits: usize) -> Self { let coeffs = ifft(self).lde(rate_bits); - fft(coeffs) + fft_with_options(coeffs, Some(rate_bits), None) } pub fn degree(&self) -> usize { diff --git a/src/util/mod.rs b/src/util/mod.rs index f901b0af..8fd60d53 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -49,13 +49,13 @@ pub(crate) fn transpose(matrix: &[Vec]) -> Vec> { } /// Permutes `arr` such that each index is mapped to its reverse in binary. -pub(crate) fn reverse_index_bits(arr: Vec) -> Vec { +pub(crate) fn reverse_index_bits(arr: Vec) -> Vec { let n = arr.len(); let n_power = log2_strict(n); let mut result = Vec::with_capacity(n); for i in 0..n { - result.push(arr[reverse_bits(i, n_power)].clone()); + result.push(arr[reverse_bits(i, n_power)]); } result } @@ -73,12 +73,11 @@ pub(crate) fn reverse_index_bits_in_place(arr: &mut Vec) { } pub(crate) fn reverse_bits(n: usize, num_bits: usize) -> usize { - let mut result = 0; - for i in 0..num_bits { - let i_rev = num_bits - i - 1; - result |= (n >> i & 1) << i_rev; - } - result + // NB: The only reason we need overflowing_shr() here as opposed + // to plain '>>' is to accommodate the case n == num_bits == 0, + // which would become `0 >> 64`. Rust thinks that any shift of 64 + // bits causes overflow, even when the argument is zero. + n.reverse_bits().overflowing_shr(usize::BITS - num_bits as u32).0 } #[cfg(test)] From 3959cec180bb95477117feef3b31872199e4f9a0 Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Wed, 30 Jun 2021 21:59:10 -0700 Subject: [PATCH 11/21] mutable borrow fix --- src/gadgets/insert.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs index 4b6a15b1..c14dcf8d 100644 --- a/src/gadgets/insert.rs +++ b/src/gadgets/insert.rs @@ -21,7 +21,8 @@ impl, const D: usize> CircuitBuilder { /// Evaluates to 1 if `x` and `y` are equal, 0 otherwise. pub fn is_equal(&mut self, x: Target, y: Target) -> Target { - self.is_zero(self.sub(x, y)) + let difference = self.sub(x, y); + self.is_zero(difference) } /// Inserts a `Target` in a vector at a non-deterministic index. This is done by rotating to the From 8de59c2a84d404c7a39bfed2d05719820f285212 Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Wed, 30 Jun 2021 21:59:18 -0700 Subject: [PATCH 12/21] cargo fmt --- src/field/fft.rs | 76 +++++++++++++++++++++--------------- src/field/field_testing.rs | 2 +- src/gadgets/insert.rs | 13 ++---- src/generator.rs | 7 +--- src/polynomial/polynomial.rs | 5 +-- src/util/mod.rs | 4 +- 6 files changed, 56 insertions(+), 51 deletions(-) diff --git a/src/field/fft.rs b/src/field/fft.rs index af5c05a7..fa65a5ea 100644 --- a/src/field/fft.rs +++ b/src/field/fft.rs @@ -6,7 +6,10 @@ use crate::util::{log2_strict, reverse_index_bits}; // TODO: Should really do some "dynamic" dispatch to handle the // different FFT algos rather than C-style enum dispatch. -enum FftStrategy { Classic, Unrolled } +enum FftStrategy { + Classic, + Unrolled, +} const FFT_STRATEGY: FftStrategy = FftStrategy::Classic; @@ -33,7 +36,6 @@ fn fft_classic_root_table(n: usize) -> FftRootTable { root_table } - fn fft_unrolled_root_table(n: usize) -> FftRootTable { // Precompute a table of the roots of unity used in the main // loops. @@ -67,18 +69,20 @@ fn fft_unrolled_root_table(n: usize) -> FftRootTable { fn fft_dispatch( input: Vec, zero_factor: Option, - root_table: Option> + root_table: Option>, ) -> Vec { let n = input.len(); match FFT_STRATEGY { - FftStrategy::Classic - => fft_classic(input, - zero_factor.unwrap_or(0), - root_table.unwrap_or_else(|| fft_classic_root_table(n))), - FftStrategy::Unrolled - => fft_unrolled(input, - zero_factor.unwrap_or(0), - root_table.unwrap_or_else(|| fft_unrolled_root_table(n))) + FftStrategy::Classic => fft_classic( + input, + zero_factor.unwrap_or(0), + root_table.unwrap_or_else(|| fft_classic_root_table(n)), + ), + FftStrategy::Unrolled => fft_unrolled( + input, + zero_factor.unwrap_or(0), + root_table.unwrap_or_else(|| fft_unrolled_root_table(n)), + ), } } @@ -91,10 +95,12 @@ pub fn fft(poly: PolynomialCoeffs) -> PolynomialValues { pub fn fft_with_options( poly: PolynomialCoeffs, zero_factor: Option, - root_table: Option> + root_table: Option>, ) -> PolynomialValues { let PolynomialCoeffs { coeffs } = poly; - PolynomialValues { values: fft_dispatch(coeffs, zero_factor, root_table) } + PolynomialValues { + values: fft_dispatch(coeffs, zero_factor, root_table), + } } #[inline] @@ -105,7 +111,7 @@ pub fn ifft(poly: PolynomialValues) -> PolynomialCoeffs { pub fn ifft_with_options( poly: PolynomialValues, zero_factor: Option, - root_table: Option> + root_table: Option>, ) -> PolynomialCoeffs { let n = poly.len(); let lg_n = log2_strict(n); @@ -136,7 +142,7 @@ pub fn ifft_with_options( pub(crate) fn fft_classic( input: Vec, r: usize, - root_table: FftRootTable + root_table: FftRootTable, ) -> Vec { let mut values = reverse_index_bits(input); @@ -144,7 +150,11 @@ pub(crate) fn fft_classic( let lg_n = log2_strict(n); if root_table.len() != lg_n { - panic!("Expected root table of length {}, but it was {}.", lg_n, root_table.len()); + panic!( + "Expected root table of length {}, but it was {}.", + lg_n, + root_table.len() + ); } // After reverse_index_bits, the only non-zero elements of values @@ -154,7 +164,8 @@ pub(crate) fn fft_classic( // element i*2^r with the value at i*2^r. This corresponds to the // first r rounds of the FFT when there are 2^r zeros at the end // of the original input. - if r > 0 { // if r == 0 then this loop is a noop. + if r > 0 { + // if r == 0 then this loop is a noop. let mask = !((1 << r) - 1); for i in 0..n { values[i] = values[i & mask]; @@ -162,7 +173,7 @@ pub(crate) fn fft_classic( } let mut m = 1 << (r + 1); - for lg_m in (r+1)..=lg_n { + for lg_m in (r + 1)..=lg_n { let half_m = m / 2; for k in (0..n).step_by(m) { for j in 0..half_m { @@ -185,11 +196,7 @@ pub(crate) fn fft_classic( /// The parameter r signifies that the first 1/2^r of the entries of /// input may be non-zero, but the last 1 - 1/2^r entries are /// definitely zero. -fn fft_unrolled( - input: Vec, - r_orig: usize, - root_table: FftRootTable -) -> Vec { +fn fft_unrolled(input: Vec, r_orig: usize, root_table: FftRootTable) -> Vec { let n = input.len(); let lg_n = log2_strict(input.len()); @@ -197,7 +204,7 @@ fn fft_unrolled( // FFT of a constant polynomial (including zero) is itself. if n < 2 { - return values + return values; } // The 'm' corresponds to the specialisation from the 'm' in the @@ -206,7 +213,8 @@ fn fft_unrolled( // (See comment in fft_classic near same code.) let mut r = r_orig; let mut m = 1 << r; - if r > 0 { // if r == 0 then this loop is a noop. + if r > 0 { + // if r == 0 then this loop is a noop. let mask = !((1 << r) - 1); for i in 0..n { values[i] = values[i & mask]; @@ -225,11 +233,15 @@ fn fft_unrolled( } if n == 2 { - return values + return values; } if root_table.len() != (lg_n - 1) { - panic!("Expected root table of length {}, but it was {}.", lg_n, root_table.len()); + panic!( + "Expected root table of length {}, but it was {}.", + lg_n, + root_table.len() + ); } // m = 2 @@ -253,7 +265,7 @@ fn fft_unrolled( // m >= 4 for lg_m in r..lg_n { - for k in (0..n).step_by(2*m) { + for k in (0..n).step_by(2 * m) { // Unrolled the commented loop by groups of 4 and // rearranged the lines. Improves runtime by about // 10%. @@ -294,11 +306,10 @@ fn fft_unrolled( values } - #[cfg(test)] mod tests { use crate::field::crandall_field::CrandallField; - use crate::field::fft::{fft, ifft, fft_with_options}; + use crate::field::fft::{fft, fft_with_options, ifft}; use crate::field::field::Field; use crate::polynomial::polynomial::{PolynomialCoeffs, PolynomialValues}; use crate::util::{log2_ceil, log2_strict}; @@ -328,7 +339,10 @@ mod tests { for r in 0..4 { // expand ceofficients by factor 2^r by filling with zeros let zero_tail = coefficients.clone().lde(r); - assert_eq!(fft(zero_tail.clone()), fft_with_options(zero_tail, Some(r), None)); + assert_eq!( + fft(zero_tail.clone()), + fft_with_options(zero_tail, Some(r), None) + ); } } diff --git a/src/field/field_testing.rs b/src/field/field_testing.rs index 7190684f..1f5bff6f 100644 --- a/src/field/field_testing.rs +++ b/src/field/field_testing.rs @@ -323,7 +323,7 @@ macro_rules! test_arithmetic { let v = ::PrimeField::TWO_ADICITY; - for e in [0, 1, 2, 3, 4, v - 2, v - 1, v, v + 1, v + 2, 123*v] { + for e in [0, 1, 2, 3, 4, v - 2, v - 1, v, v + 1, v + 2, 123 * v] { let x = F::TWO.exp(e as u64).inverse(); let y = F::inverse_2exp(e); assert_eq!(x, y); diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs index c14dcf8d..32aa250e 100644 --- a/src/gadgets/insert.rs +++ b/src/gadgets/insert.rs @@ -1,8 +1,8 @@ use crate::circuit_builder::CircuitBuilder; use crate::field::extension_field::target::ExtensionTarget; use crate::field::extension_field::Extendable; -use crate::target::Target; use crate::generator::EqualityGenerator; +use crate::target::Target; impl, const D: usize> CircuitBuilder { /// Evaluates to 1 if `x` equals zero, 0 otherwise. @@ -10,11 +10,7 @@ impl, const D: usize> CircuitBuilder { let m = todo!(); let y = todo!(); - self.add_generator(EqualityGenerator { - x, - m, - y, - }); + self.add_generator(EqualityGenerator { x, m, y }); y } @@ -39,15 +35,14 @@ impl, const D: usize> CircuitBuilder { for i in 0..v.len() { let one = self.one(); - + let cur_index = self.constant(F::from_canonical_usize(i)); let insert_here = self.is_equal(cur_index, index); let mut new_item = self.zero_extension(); new_item = self.scalar_mul_add_extension(insert_here, element, new_item); if i > 0 { - new_item = - self.scalar_mul_add_extension(already_inserted, v[i - 1], new_item); + new_item = self.scalar_mul_add_extension(already_inserted, v[i - 1], new_item); } already_inserted = self.add(already_inserted, insert_here); diff --git a/src/generator.rs b/src/generator.rs index 130ca0d0..64f21604 100644 --- a/src/generator.rs +++ b/src/generator.rs @@ -131,7 +131,6 @@ impl SimpleGenerator for RandomValueGenerator { } } - /// A generator for including a random value pub(crate) struct EqualityGenerator { pub(crate) x: Target, @@ -147,11 +146,7 @@ impl SimpleGenerator for EqualityGenerator { fn run_once(&self, witness: &PartialWitness) -> PartialWitness { let x_value = witness.get_target(self.x); - let y_value = if x_value == F::ZERO { - F::ZERO - } else { - F::ONE - }; + let y_value = if x_value == F::ZERO { F::ZERO } else { F::ONE }; let m_value = if x_value == F::ZERO { F::ONE diff --git a/src/polynomial/polynomial.rs b/src/polynomial/polynomial.rs index 5f295030..aa06d641 100644 --- a/src/polynomial/polynomial.rs +++ b/src/polynomial/polynomial.rs @@ -1,13 +1,12 @@ -use std::time::Instant; - use std::cmp::max; use std::iter::Sum; use std::ops::{Add, AddAssign, Mul, MulAssign, Sub, SubAssign}; +use std::time::Instant; use anyhow::{ensure, Result}; use crate::field::extension_field::Extendable; -use crate::field::fft::{fft, ifft, fft_with_options}; +use crate::field::fft::{fft, fft_with_options, ifft}; use crate::field::field::Field; use crate::util::log2_strict; diff --git a/src/util/mod.rs b/src/util/mod.rs index 8fd60d53..ee3b8440 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -77,7 +77,9 @@ pub(crate) fn reverse_bits(n: usize, num_bits: usize) -> usize { // to plain '>>' is to accommodate the case n == num_bits == 0, // which would become `0 >> 64`. Rust thinks that any shift of 64 // bits causes overflow, even when the argument is zero. - n.reverse_bits().overflowing_shr(usize::BITS - num_bits as u32).0 + n.reverse_bits() + .overflowing_shr(usize::BITS - num_bits as u32) + .0 } #[cfg(test)] From 95a875e28d213741d9b263ac7c4efe8cb6c8340b Mon Sep 17 00:00:00 2001 From: Daniel Lubarov Date: Thu, 1 Jul 2021 08:12:12 -0700 Subject: [PATCH 13/21] Allow virtual targets to be routed (#84) As in plonky1. The semantics of virtual targets in plonky1 were rather weird, but I think it's somewhat better here, since we already separate `generate_copy` and `assert_equal` methods. Users now make more of an explicit choice -- they can use a `VirtualTarget` for the witness generation only using `generate_copy`, or they can involve it in copy constraints. --- src/circuit_builder.rs | 28 ++++++++++++++-------------- src/gadgets/split_join.rs | 6 +++--- src/permutation_argument.rs | 2 +- src/target.rs | 7 +++++-- src/witness.rs | 2 +- 5 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/circuit_builder.rs b/src/circuit_builder.rs index 7ce4adb3..06c554e3 100644 --- a/src/circuit_builder.rs +++ b/src/circuit_builder.rs @@ -36,7 +36,7 @@ pub struct CircuitBuilder, const D: usize> { /// The next available index for a public input. public_input_index: usize, - /// The next available index for a VirtualAdviceTarget. + /// The next available index for a `VirtualTarget`. virtual_target_index: usize, copy_constraints: Vec<(Target, Target)>, @@ -77,22 +77,18 @@ impl, const D: usize> CircuitBuilder { (0..n).map(|_i| self.add_public_input()).collect() } - /// Adds a new "virtual" advice target. This is not an actual wire in the witness, but just a - /// target that help facilitate witness generation. In particular, a generator can assign a - /// values to a virtual target, which can then be copied to other (virtual or concrete) targets - /// via `generate_copy`. When we generate the final witness (a grid of wire values), these - /// virtual targets will go away. - /// - /// Since virtual targets are not part of the actual permutation argument, they cannot be used - /// with `assert_equal`. - pub fn add_virtual_advice_target(&mut self) -> Target { + /// Adds a new "virtual" target. This is not an actual wire in the witness, but just a target + /// that help facilitate witness generation. In particular, a generator can assign a values to a + /// virtual target, which can then be copied to other (virtual or concrete) targets. When we + /// generate the final witness (a grid of wire values), these virtual targets will go away. + pub fn add_virtual_target(&mut self) -> Target { let index = self.virtual_target_index; self.virtual_target_index += 1; - Target::VirtualAdviceTarget { index } + Target::VirtualTarget { index } } - pub fn add_virtual_advice_targets(&mut self, n: usize) -> Vec { - (0..n).map(|_i| self.add_virtual_advice_target()).collect() + pub fn add_virtual_targets(&mut self, n: usize) -> Vec { + (0..n).map(|_i| self.add_virtual_target()).collect() } pub fn add_gate_no_constants(&mut self, gate_type: GateRef) -> usize { @@ -368,7 +364,11 @@ impl, const D: usize> CircuitBuilder { } for index in 0..self.public_input_index { - target_partitions.add_partition(Target::PublicInput { index }) + target_partitions.add_partition(Target::PublicInput { index }); + } + + for index in 0..self.virtual_target_index { + target_partitions.add_partition(Target::VirtualTarget { index }); } for &(a, b) in &self.copy_constraints { diff --git a/src/gadgets/split_join.rs b/src/gadgets/split_join.rs index 5eb60148..3a2c27f4 100644 --- a/src/gadgets/split_join.rs +++ b/src/gadgets/split_join.rs @@ -9,14 +9,14 @@ use crate::wire::Wire; use crate::witness::PartialWitness; impl, const D: usize> CircuitBuilder { - /// Split the given integer into a list of virtual advice targets, where each one represents a - /// bit of the integer, with little-endian ordering. + /// Split the given integer into a list of virtual targets, where each one represents a bit of + /// the integer, with little-endian ordering. /// /// Note that this only handles witness generation; it does not enforce that the decomposition /// is correct. The output should be treated as a "purported" decomposition which must be /// enforced elsewhere. pub(crate) fn split_le_virtual(&mut self, integer: Target, num_bits: usize) -> Vec { - let bit_targets = self.add_virtual_advice_targets(num_bits); + let bit_targets = self.add_virtual_targets(num_bits); self.add_generator(SplitGenerator { integer, bits: bit_targets.clone(), diff --git a/src/permutation_argument.rs b/src/permutation_argument.rs index 54436ecb..cc202a95 100644 --- a/src/permutation_argument.rs +++ b/src/permutation_argument.rs @@ -57,7 +57,7 @@ impl TargetPartitions { } pub fn to_wire_partitions(&self) -> WirePartitions { - // Here we just drop all CircuitInputs, leaving all GateInputs. + // Here we keep just the Wire targets, filtering out everything else. let mut partitions = Vec::new(); let mut indices = HashMap::new(); diff --git a/src/target.rs b/src/target.rs index 8aec0b5a..e765f7eb 100644 --- a/src/target.rs +++ b/src/target.rs @@ -8,7 +8,10 @@ use crate::wire::Wire; pub enum Target { Wire(Wire), PublicInput { index: usize }, - VirtualAdviceTarget { index: usize }, + /// A target that doesn't have any inherent location in the witness (but it can be copied to + /// another target that does). This is useful for representing intermediate values in witness + /// generation. + VirtualTarget { index: usize }, } impl Target { @@ -20,7 +23,7 @@ impl Target { match self { Target::Wire(wire) => wire.is_routable(config), Target::PublicInput { .. } => true, - Target::VirtualAdviceTarget { .. } => false, + Target::VirtualTarget { .. } => true, } } diff --git a/src/witness.rs b/src/witness.rs index 681049d0..863ee7dd 100644 --- a/src/witness.rs +++ b/src/witness.rs @@ -30,7 +30,7 @@ impl Witness { F: Extendable, { for &(a, b) in copy_constraints { - // TODO: Take care of public inputs once they land. + // TODO: Take care of public inputs once they land, and virtual targets. if let ( Target::Wire(Wire { gate: a_gate, From f4ca0df85d0c3b54741dd3a9b7d91cfd9837f59c Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Thu, 1 Jul 2021 10:35:41 -0700 Subject: [PATCH 14/21] comments and renaming --- src/gadgets/insert.rs | 4 ++-- src/generator.rs | 26 +++++++++++++------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs index 32aa250e..78bf54cc 100644 --- a/src/gadgets/insert.rs +++ b/src/gadgets/insert.rs @@ -1,7 +1,7 @@ use crate::circuit_builder::CircuitBuilder; use crate::field::extension_field::target::ExtensionTarget; use crate::field::extension_field::Extendable; -use crate::generator::EqualityGenerator; +use crate::generator::EqualsZeroGenerator; use crate::target::Target; impl, const D: usize> CircuitBuilder { @@ -10,7 +10,7 @@ impl, const D: usize> CircuitBuilder { let m = todo!(); let y = todo!(); - self.add_generator(EqualityGenerator { x, m, y }); + self.add_generator(EqualsZeroGenerator { x, m, y }); y } diff --git a/src/generator.rs b/src/generator.rs index 64f21604..f54b820e 100644 --- a/src/generator.rs +++ b/src/generator.rs @@ -131,32 +131,32 @@ impl SimpleGenerator for RandomValueGenerator { } } -/// A generator for including a random value -pub(crate) struct EqualityGenerator { - pub(crate) x: Target, - pub(crate) m: Target, - pub(crate) y: Target, +/// A generator for testing if a value equals zero +pub(crate) struct EqualsZeroGenerator { + pub(crate) to_test: Target, + pub(crate) dummy: Target, + pub(crate) is_zero: Target, } -impl SimpleGenerator for EqualityGenerator { +impl SimpleGenerator for EqualsZeroGenerator { fn dependencies(&self) -> Vec { - vec![self.x] + vec![self.to_test] } fn run_once(&self, witness: &PartialWitness) -> PartialWitness { - let x_value = witness.get_target(self.x); + let to_test_value = witness.get_target(self.to_test); - let y_value = if x_value == F::ZERO { F::ZERO } else { F::ONE }; + let is_zero_value = if to_test_value == F::ZERO { F::ZERO } else { F::ONE }; - let m_value = if x_value == F::ZERO { + let dummy_value = if to_test_value == F::ZERO { F::ONE } else { - x_value.inverse() + to_test_value.inverse() }; let mut witness = PartialWitness::new(); - witness.set_target(self.m, m_value); - witness.set_target(self.y, y_value); + witness.set_target(self.dummy, dummy_value); + witness.set_target(self.is_zero, is_zero_value); witness } } From 702a098054f96c26e45c797aa03f4a9661c7cb0e Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Thu, 1 Jul 2021 10:36:31 -0700 Subject: [PATCH 15/21] cargo fmt --- src/generator.rs | 6 +++++- src/target.rs | 8 ++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/generator.rs b/src/generator.rs index f54b820e..6bc12011 100644 --- a/src/generator.rs +++ b/src/generator.rs @@ -146,7 +146,11 @@ impl SimpleGenerator for EqualsZeroGenerator { fn run_once(&self, witness: &PartialWitness) -> PartialWitness { let to_test_value = witness.get_target(self.to_test); - let is_zero_value = if to_test_value == F::ZERO { F::ZERO } else { F::ONE }; + let is_zero_value = if to_test_value == F::ZERO { + F::ZERO + } else { + F::ONE + }; let dummy_value = if to_test_value == F::ZERO { F::ONE diff --git a/src/target.rs b/src/target.rs index e765f7eb..52be8b5a 100644 --- a/src/target.rs +++ b/src/target.rs @@ -7,11 +7,15 @@ use crate::wire::Wire; #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug)] pub enum Target { Wire(Wire), - PublicInput { index: usize }, + PublicInput { + index: usize, + }, /// A target that doesn't have any inherent location in the witness (but it can be copied to /// another target that does). This is useful for representing intermediate values in witness /// generation. - VirtualTarget { index: usize }, + VirtualTarget { + index: usize, + }, } impl Target { From 515373653d155d39120707a7f7341f7292d90838 Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Thu, 1 Jul 2021 10:38:11 -0700 Subject: [PATCH 16/21] fix --- src/gadgets/insert.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs index 78bf54cc..c58e80b9 100644 --- a/src/gadgets/insert.rs +++ b/src/gadgets/insert.rs @@ -10,7 +10,11 @@ impl, const D: usize> CircuitBuilder { let m = todo!(); let y = todo!(); - self.add_generator(EqualsZeroGenerator { x, m, y }); + self.add_generator(EqualsZeroGenerator { + to_test: x, + dummy: m, + is_zero: y + }); y } From d84b9ec8cb94aec72abbe4c29a3feaecd2c982e5 Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Thu, 1 Jul 2021 10:47:13 -0700 Subject: [PATCH 17/21] is_zero function --- src/gadgets/insert.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs index c58e80b9..ca7149bf 100644 --- a/src/gadgets/insert.rs +++ b/src/gadgets/insert.rs @@ -7,13 +7,18 @@ use crate::target::Target; impl, const D: usize> CircuitBuilder { /// Evaluates to 1 if `x` equals zero, 0 otherwise. pub fn is_zero(&mut self, x: Target) -> Target { - let m = todo!(); - let y = todo!(); + let m = self.add_virtual_target(); + let y = self.mul(x, m); + + let one = self.one(); + let diff = self.sub(one, y); + let prod = self.mul(diff, x); + self.assert_zero(prod); self.add_generator(EqualsZeroGenerator { to_test: x, dummy: m, - is_zero: y + is_zero: y, }); y From 519533d4b775b8f1a7cf4ccb1e4fa6a39b26aadf Mon Sep 17 00:00:00 2001 From: Daniel Lubarov Date: Thu, 1 Jul 2021 10:53:42 -0700 Subject: [PATCH 18/21] Benchmark tweaks (#83) - Configure FRI with a list of arities that's more appropriate for a 2^14 instance. The previous config resulted in a huge final polynomial. - Log the blinding factors, and other logging tweaks. --- src/bin/bench_recursion.rs | 4 +--- src/circuit_builder.rs | 8 ++++++-- src/gadgets/arithmetic.rs | 1 - src/gadgets/arithmetic_extension.rs | 2 +- src/gates/gate_tree.rs | 2 +- src/polynomial/polynomial.rs | 4 +--- src/util/timing.rs | 2 +- 7 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/bin/bench_recursion.rs b/src/bin/bench_recursion.rs index 0f1b9783..b55ee841 100644 --- a/src/bin/bench_recursion.rs +++ b/src/bin/bench_recursion.rs @@ -21,8 +21,6 @@ fn main() { } fn bench_prove, const D: usize>() { - let gmimc_gate = GMiMCGate::::with_automatic_constants(); - let config = CircuitConfig { num_wires: 134, num_routed_wires: 27, @@ -32,7 +30,7 @@ fn bench_prove, const D: usize>() { fri_config: FriConfig { proof_of_work_bits: 1, rate_bits: 3, - reduction_arity_bits: vec![1, 1, 1, 1], + reduction_arity_bits: vec![2, 2, 2, 2, 2], num_query_rounds: 1, }, }; diff --git a/src/circuit_builder.rs b/src/circuit_builder.rs index 06c554e3..2d00b403 100644 --- a/src/circuit_builder.rs +++ b/src/circuit_builder.rs @@ -277,6 +277,10 @@ impl, const D: usize> CircuitBuilder { fn blind_and_pad(&mut self) { let (regular_poly_openings, z_openings) = self.blinding_counts(); + info!( + "Adding {} blinding terms for witness polynomials, and {}*2 for Z polynomials", + regular_poly_openings, z_openings + ); let num_routed_wires = self.config.num_routed_wires; let num_wires = self.config.num_wires; @@ -383,12 +387,12 @@ impl, const D: usize> CircuitBuilder { pub fn build(mut self) -> CircuitData { let start = Instant::now(); info!( - "degree before blinding & padding: {}", + "Degree before blinding & padding: {}", self.gate_instances.len() ); self.blind_and_pad(); let degree = self.gate_instances.len(); - info!("degree after blinding & padding: {}", degree); + info!("Degree after blinding & padding: {}", degree); let gates = self.gates.iter().cloned().collect(); let (gate_tree, max_filtered_constraint_degree, num_constants) = Tree::from_gates(gates); diff --git a/src/gadgets/arithmetic.rs b/src/gadgets/arithmetic.rs index bda70624..5c328362 100644 --- a/src/gadgets/arithmetic.rs +++ b/src/gadgets/arithmetic.rs @@ -1,7 +1,6 @@ use crate::circuit_builder::CircuitBuilder; use crate::field::extension_field::Extendable; use crate::target::Target; -use crate::util::bits_u64; impl, const D: usize> CircuitBuilder { /// Computes `-x`. diff --git a/src/gadgets/arithmetic_extension.rs b/src/gadgets/arithmetic_extension.rs index ce53c59f..4f7b1e14 100644 --- a/src/gadgets/arithmetic_extension.rs +++ b/src/gadgets/arithmetic_extension.rs @@ -6,7 +6,7 @@ use num::Integer; use crate::circuit_builder::CircuitBuilder; use crate::field::extension_field::target::{ExtensionAlgebraTarget, ExtensionTarget}; -use crate::field::extension_field::{Extendable, FieldExtension, OEF}; +use crate::field::extension_field::{Extendable, OEF}; use crate::field::field::Field; use crate::gates::arithmetic::ArithmeticExtensionGate; use crate::generator::SimpleGenerator; diff --git a/src/gates/gate_tree.rs b/src/gates/gate_tree.rs index bf56a690..5c2e084e 100644 --- a/src/gates/gate_tree.rs +++ b/src/gates/gate_tree.rs @@ -86,7 +86,7 @@ impl, const D: usize> Tree> { } } info!( - "Found tree with max degree {} and {} constants wires in {}s.", + "Found tree with max degree {} and {} constants wires in {:.4}s.", best_degree, best_num_constants, timer.elapsed().as_secs_f32() diff --git a/src/polynomial/polynomial.rs b/src/polynomial/polynomial.rs index 5f295030..81d07b8f 100644 --- a/src/polynomial/polynomial.rs +++ b/src/polynomial/polynomial.rs @@ -1,5 +1,3 @@ -use std::time::Instant; - use std::cmp::max; use std::iter::Sum; use std::ops::{Add, AddAssign, Mul, MulAssign, Sub, SubAssign}; @@ -7,7 +5,7 @@ use std::ops::{Add, AddAssign, Mul, MulAssign, Sub, SubAssign}; use anyhow::{ensure, Result}; use crate::field::extension_field::Extendable; -use crate::field::fft::{fft, ifft, fft_with_options}; +use crate::field::fft::{fft, fft_with_options, ifft}; use crate::field::field::Field; use crate::util::log2_strict; diff --git a/src/util/timing.rs b/src/util/timing.rs index 17136f36..6f27a9d1 100644 --- a/src/util/timing.rs +++ b/src/util/timing.rs @@ -7,7 +7,7 @@ macro_rules! timed { let timer = Instant::now(); let res = $a; - info!("{:.3}s {}", timer.elapsed().as_secs_f32(), $msg); + info!("{:.4}s {}", timer.elapsed().as_secs_f32(), $msg); res }}; } From efe39f2d638dd5cf3bf5f5162b502d9df1468432 Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Thu, 1 Jul 2021 11:21:33 -0700 Subject: [PATCH 19/21] fixed naming (zero --> nonzero), and other fixes --- src/gadgets/insert.rs | 26 ++++++++++++++++---------- src/generator.rs | 12 ++---------- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs index ca7149bf..64260209 100644 --- a/src/gadgets/insert.rs +++ b/src/gadgets/insert.rs @@ -1,33 +1,39 @@ use crate::circuit_builder::CircuitBuilder; use crate::field::extension_field::target::ExtensionTarget; use crate::field::extension_field::Extendable; -use crate::generator::EqualsZeroGenerator; +use crate::generator::NonzeroTestGenerator; use crate::target::Target; impl, const D: usize> CircuitBuilder { - /// Evaluates to 1 if `x` equals zero, 0 otherwise. - pub fn is_zero(&mut self, x: Target) -> Target { + /// Evaluates to 0 if `x` equals zero, 1 otherwise. + pub fn is_nonzero(&mut self, x: Target) -> Target { + // Dummy variable. let m = self.add_virtual_target(); + + // The prover sets the dummy variable to 0 if x == 0 and to 1/x otherwise. + self.add_generator(NonzeroTestGenerator { + to_test: x, + dummy: m, + }); + + // Evaluates to (0) * (0) = 0 if x == 0 and (x) * (1/x) = 1 otherwise. let y = self.mul(x, m); + // Enforce that (1 - y) * x == 0. let one = self.one(); let diff = self.sub(one, y); let prod = self.mul(diff, x); self.assert_zero(prod); - self.add_generator(EqualsZeroGenerator { - to_test: x, - dummy: m, - is_zero: y, - }); - y } /// Evaluates to 1 if `x` and `y` are equal, 0 otherwise. pub fn is_equal(&mut self, x: Target, y: Target) -> Target { let difference = self.sub(x, y); - self.is_zero(difference) + let not_equal = self.is_nonzero(difference); + let one = self.one(); + self.sub(one, not_equal) } /// Inserts a `Target` in a vector at a non-deterministic index. This is done by rotating to the diff --git a/src/generator.rs b/src/generator.rs index 6bc12011..3795fdfa 100644 --- a/src/generator.rs +++ b/src/generator.rs @@ -132,13 +132,12 @@ impl SimpleGenerator for RandomValueGenerator { } /// A generator for testing if a value equals zero -pub(crate) struct EqualsZeroGenerator { +pub(crate) struct NonzeroTestGenerator { pub(crate) to_test: Target, pub(crate) dummy: Target, - pub(crate) is_zero: Target, } -impl SimpleGenerator for EqualsZeroGenerator { +impl SimpleGenerator for NonzeroTestGenerator { fn dependencies(&self) -> Vec { vec![self.to_test] } @@ -146,12 +145,6 @@ impl SimpleGenerator for EqualsZeroGenerator { fn run_once(&self, witness: &PartialWitness) -> PartialWitness { let to_test_value = witness.get_target(self.to_test); - let is_zero_value = if to_test_value == F::ZERO { - F::ZERO - } else { - F::ONE - }; - let dummy_value = if to_test_value == F::ZERO { F::ONE } else { @@ -160,7 +153,6 @@ impl SimpleGenerator for EqualsZeroGenerator { let mut witness = PartialWitness::new(); witness.set_target(self.dummy, dummy_value); - witness.set_target(self.is_zero, is_zero_value); witness } } From 39b22a6cab780adf2e99c14218584ae3b4c8defe Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Thu, 1 Jul 2021 12:00:56 -0700 Subject: [PATCH 20/21] addressed nits --- src/gadgets/insert.rs | 5 ++++- src/generator.rs | 4 +--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs index 64260209..7a1ee98d 100644 --- a/src/gadgets/insert.rs +++ b/src/gadgets/insert.rs @@ -6,11 +6,14 @@ use crate::target::Target; impl, const D: usize> CircuitBuilder { /// Evaluates to 0 if `x` equals zero, 1 otherwise. + /// From section 2 of https://github.com/mir-protocol/r1cs-workshop/blob/master/workshop.pdf, + /// based on an idea from https://eprint.iacr.org/2012/598.pdf. pub fn is_nonzero(&mut self, x: Target) -> Target { // Dummy variable. let m = self.add_virtual_target(); - // The prover sets the dummy variable to 0 if x == 0 and to 1/x otherwise. + // The prover sets this the dummy variable to 1/x if x != 0, or to an arbitrary value if + // x == 0. self.add_generator(NonzeroTestGenerator { to_test: x, dummy: m, diff --git a/src/generator.rs b/src/generator.rs index 3795fdfa..a47c5267 100644 --- a/src/generator.rs +++ b/src/generator.rs @@ -151,8 +151,6 @@ impl SimpleGenerator for NonzeroTestGenerator { to_test_value.inverse() }; - let mut witness = PartialWitness::new(); - witness.set_target(self.dummy, dummy_value); - witness + PartialWitness::singleton_target(self.dummy, dummy_value) } } From 3d53201538ca8faef39c80e15f229847e05fdd94 Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Thu, 1 Jul 2021 17:43:22 -0700 Subject: [PATCH 21/21] save a gate with arithmetic --- src/gadgets/insert.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs index 7a1ee98d..1f69cb24 100644 --- a/src/gadgets/insert.rs +++ b/src/gadgets/insert.rs @@ -23,9 +23,7 @@ impl, const D: usize> CircuitBuilder { let y = self.mul(x, m); // Enforce that (1 - y) * x == 0. - let one = self.one(); - let diff = self.sub(one, y); - let prod = self.mul(diff, x); + let prod = self.arithmetic(F::NEG_ONE, x, y, F::ONE, x); self.assert_zero(prod); y