From dfbd5e3332084281e484e421a2c34451b81f3049 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 21 Jul 2021 10:19:35 -0600 Subject: [PATCH 01/19] Distinguish between selectors and all other columns in layouter using RegionColumn. --- src/circuit/floor_planner/single_pass.rs | 6 +-- src/circuit/floor_planner/v1.rs | 6 +-- src/circuit/floor_planner/v1/strategy.rs | 25 ++++++++---- src/circuit/layouter.rs | 52 ++++++++++++++++++++---- 4 files changed, 67 insertions(+), 22 deletions(-) diff --git a/src/circuit/floor_planner/single_pass.rs b/src/circuit/floor_planner/single_pass.rs index 5b955290..4a2e4961 100644 --- a/src/circuit/floor_planner/single_pass.rs +++ b/src/circuit/floor_planner/single_pass.rs @@ -7,11 +7,11 @@ use ff::Field; use crate::{ circuit::{ - layouter::{RegionLayouter, RegionShape}, + layouter::{RegionColumn, RegionLayouter, RegionShape}, Cell, Layouter, Region, RegionIndex, RegionStart, }, plonk::{ - Advice, Any, Assigned, Assignment, Circuit, Column, Error, Fixed, FloorPlanner, Instance, + Advice, Assigned, Assignment, Circuit, Column, Error, Fixed, FloorPlanner, Instance, Selector, }, }; @@ -43,7 +43,7 @@ pub struct SingleChipLayouter<'a, F: Field, CS: Assignment + 'a> { /// Stores the starting row for each region. regions: Vec, /// Stores the first empty row for each column. - columns: HashMap, usize>, + columns: HashMap, _marker: PhantomData, } diff --git a/src/circuit/floor_planner/v1.rs b/src/circuit/floor_planner/v1.rs index 17adeedd..823f5b56 100644 --- a/src/circuit/floor_planner/v1.rs +++ b/src/circuit/floor_planner/v1.rs @@ -4,11 +4,11 @@ use ff::Field; use crate::{ circuit::{ - layouter::{RegionLayouter, RegionShape}, + layouter::{RegionColumn, RegionLayouter, RegionShape}, Cell, Layouter, Region, RegionIndex, RegionStart, }, plonk::{ - Advice, Assigned, Assignment, Circuit, Column, Error, Fixed, FloorPlanner, Instance, + Advice, Any, Assigned, Assignment, Circuit, Column, Error, Fixed, FloorPlanner, Instance, Selector, }, }; @@ -89,7 +89,7 @@ impl FloorPlanner for V1 { ( c, column_allocations - .get(&c.into()) + .get(&Column::::from(c).into()) .cloned() .unwrap_or_default(), ) diff --git a/src/circuit/floor_planner/v1/strategy.rs b/src/circuit/floor_planner/v1/strategy.rs index 090aceec..8b43e205 100644 --- a/src/circuit/floor_planner/v1/strategy.rs +++ b/src/circuit/floor_planner/v1/strategy.rs @@ -4,11 +4,8 @@ use std::{ ops::Range, }; -use super::RegionShape; -use crate::{ - circuit::RegionStart, - plonk::{Any, Column}, -}; +use super::{RegionColumn, RegionShape}; +use crate::{circuit::RegionStart, plonk::Any}; /// A region allocated within a column. #[derive(Clone, Default, Debug, PartialEq, Eq)] @@ -102,14 +99,14 @@ impl Allocations { } /// Allocated rows within a circuit. -pub type CircuitAllocations = HashMap, Allocations>; +pub type CircuitAllocations = HashMap; /// - `start` is the current start row of the region (not of this column). /// - `slack` is the maximum number of rows the start could be moved down, taking into /// account prior columns. fn first_fit_region( column_allocations: &mut CircuitAllocations, - region_columns: &[Column], + region_columns: &[RegionColumn], region_length: usize, start: usize, slack: Option, @@ -207,7 +204,10 @@ pub fn slot_in_biggest_advice_first( let advice_cols = shape .columns() .iter() - .filter(|c| matches!(c.column_type(), Any::Advice)) + .filter(|c| match c { + RegionColumn::Column(c) => matches!(c.column_type(), Any::Advice), + _ => false, + }) .count(); // Sort by advice area (since this has the most contention). advice_cols * shape.row_count() @@ -226,23 +226,30 @@ pub fn slot_in_biggest_advice_first( #[test] fn test_slot_in() { + use crate::plonk::Column; + let regions = vec![ RegionShape { region_index: 0.into(), columns: vec![Column::new(0, Any::Advice), Column::new(1, Any::Advice)] .into_iter() + .map(|a| Column::::from(a).into()) .collect(), row_count: 15, }, RegionShape { region_index: 1.into(), - columns: vec![Column::new(2, Any::Advice)].into_iter().collect(), + columns: vec![Column::new(2, Any::Advice)] + .into_iter() + .map(|a| Column::::from(a).into()) + .collect(), row_count: 10, }, RegionShape { region_index: 2.into(), columns: vec![Column::new(2, Any::Advice), Column::new(0, Any::Advice)] .into_iter() + .map(|a| Column::::from(a).into()) .collect(), row_count: 10, }, diff --git a/src/circuit/layouter.rs b/src/circuit/layouter.rs index 10fa84d2..f020688a 100644 --- a/src/circuit/layouter.rs +++ b/src/circuit/layouter.rs @@ -110,10 +110,49 @@ pub trait RegionLayouter: fmt::Debug { #[derive(Clone, Debug)] pub struct RegionShape { pub(super) region_index: RegionIndex, - pub(super) columns: HashSet>, + pub(super) columns: HashSet, pub(super) row_count: usize, } +/// The virtual column involved in a region. This includes normal "logical" +/// columns as well as selectors that are not definite columns at this stage. +#[derive(Eq, PartialEq, Copy, Clone, Debug, Hash)] +pub enum RegionColumn { + /// Logical column + Column(Column), + /// Virtual column representing a (boolean) selector + Selector(Selector), +} + +impl From> for RegionColumn { + fn from(column: Column) -> RegionColumn { + RegionColumn::Column(column) + } +} + +impl From for RegionColumn { + fn from(selector: Selector) -> RegionColumn { + RegionColumn::Selector(selector) + } +} + +impl Ord for RegionColumn { + fn cmp(&self, other: &Self) -> cmp::Ordering { + match (self, other) { + (Self::Column(ref a), Self::Column(ref b)) => a.cmp(b), + (Self::Selector(ref a), Self::Selector(ref b)) => a.0.cmp(&b.0), + (Self::Column(_), Self::Selector(_)) => cmp::Ordering::Greater, + (Self::Selector(_), Self::Column(_)) => cmp::Ordering::Less, + } + } +} + +impl PartialOrd for RegionColumn { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + impl RegionShape { /// Create a new `RegionShape` for a region at `region_index`. pub fn new(region_index: RegionIndex) -> Self { @@ -130,7 +169,7 @@ impl RegionShape { } /// Get a reference to the set of `columns` used in a `RegionShape`. - pub fn columns(&self) -> &HashSet> { + pub fn columns(&self) -> &HashSet { &self.columns } @@ -148,8 +187,7 @@ impl RegionLayouter for RegionShape { offset: usize, ) -> Result<(), Error> { // Track the selector's fixed column as part of the region's shape. - // TODO: Avoid exposing selector internals? - self.columns.insert(selector.0.into()); + self.columns.insert((*selector).into()); self.row_count = cmp::max(self.row_count, offset + 1); Ok(()) } @@ -161,7 +199,7 @@ impl RegionLayouter for RegionShape { offset: usize, _to: &'v mut (dyn FnMut() -> Result, Error> + 'v), ) -> Result { - self.columns.insert(column.into()); + self.columns.insert(Column::::from(column).into()); self.row_count = cmp::max(self.row_count, offset + 1); Ok(Cell { @@ -190,7 +228,7 @@ impl RegionLayouter for RegionShape { advice: Column, offset: usize, ) -> Result<(Cell, Option), Error> { - self.columns.insert(advice.into()); + self.columns.insert(Column::::from(advice).into()); self.row_count = cmp::max(self.row_count, offset + 1); Ok(( @@ -210,7 +248,7 @@ impl RegionLayouter for RegionShape { offset: usize, _to: &'v mut (dyn FnMut() -> Result, Error> + 'v), ) -> Result { - self.columns.insert(column.into()); + self.columns.insert(Column::::from(column).into()); self.row_count = cmp::max(self.row_count, offset + 1); Ok(Cell { From 382cd5a7ea66ef38966e51b9dd2678ad43187a08 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 21 Jul 2021 12:55:19 -0600 Subject: [PATCH 02/19] Create actual selector columns only during an optimization pass. --- src/dev.rs | 28 +++++++--- src/plonk/circuit.rs | 102 +++++++++++++++++++++++++++++++---- src/plonk/keygen.rs | 36 ++++++++----- src/plonk/lookup/prover.rs | 2 + src/plonk/lookup/verifier.rs | 1 + src/plonk/prover.rs | 5 ++ src/plonk/verifier.rs | 1 + tests/plonk_api.rs | 2 + 8 files changed, 145 insertions(+), 32 deletions(-) diff --git a/src/dev.rs b/src/dev.rs index f1871519..d7014fb2 100644 --- a/src/dev.rs +++ b/src/dev.rs @@ -327,6 +327,8 @@ pub struct MockProver { // The instance cells in the circuit, arranged as [column][row]. instance: Vec>, + selectors: Vec>, + permutation: permutation::keygen::Assembly, // A range of available rows for assignment and copies. @@ -352,12 +354,7 @@ impl Assignment for MockProver { self.regions.push(self.current_region.take().unwrap()); } - fn enable_selector( - &mut self, - annotation: A, - selector: &Selector, - row: usize, - ) -> Result<(), Error> + fn enable_selector(&mut self, _: A, selector: &Selector, row: usize) -> Result<(), Error> where A: FnOnce() -> AR, AR: Into, @@ -376,8 +373,9 @@ impl Assignment for MockProver { .or_default() .push(row); - // Selectors are just fixed columns. - self.assign_fixed(annotation, selector.0, row, || Ok(F::one())) + self.selectors[selector.0][row] = true; + + Ok(()) } fn query_instance(&self, column: Column, row: usize) -> Result, Error> { @@ -518,6 +516,7 @@ impl MockProver { // Fixed columns contain no blinding factors. let fixed = vec![vec![CellValue::Unassigned; n]; cs.num_fixed_columns]; + let selectors = vec![vec![false; n]; cs.num_selectors]; // Advice columns contain blinding factors. let blinding_factors = cs.blinding_factors(); let usable_rows = n - (blinding_factors + 1); @@ -543,12 +542,23 @@ impl MockProver { fixed, advice, instance, + selectors, permutation, usable_rows: 0..usable_rows, }; ConcreteCircuit::FloorPlanner::synthesize(&mut prover, circuit, config, constants)?; + let (cs, selector_polys) = prover.cs.compress_selectors(prover.selectors.clone()); + prover.cs = cs; + prover.fixed.extend(selector_polys.into_iter().map(|poly| { + let mut v = vec![CellValue::Unassigned; n]; + for (v, p) in v.iter_mut().zip(&poly[..]) { + *v = CellValue::Assigned(*p); + } + v + })); + Ok(prover) } @@ -640,6 +650,7 @@ impl MockProver { gate.polynomials().iter().enumerate().filter_map( move |(poly_index, poly)| match poly.evaluate( &|scalar| Value::Real(scalar), + &|_| panic!("virtual selectors are removed during optimization"), &load(n, row, &self.cs.fixed_queries, &self.fixed), &load(n, row, &self.cs.advice_queries, &self.advice), &load_instance(n, row, &self.cs.instance_queries, &self.instance), @@ -680,6 +691,7 @@ impl MockProver { let load = |expression: &Expression, row| { expression.evaluate( &|scalar| Value::Real(scalar), + &|_| panic!("virtual selectors are removed during optimization"), &|index, _, _| { let query = self.cs.fixed_queries[index]; let column_index = query.0.index(); diff --git a/src/plonk/circuit.rs b/src/plonk/circuit.rs index 0f624c5e..ffdba644 100644 --- a/src/plonk/circuit.rs +++ b/src/plonk/circuit.rs @@ -228,7 +228,7 @@ impl TryFrom> for Column { /// } /// ``` #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] -pub struct Selector(pub(crate) Column); +pub struct Selector(pub(crate) usize); impl Selector { /// Enable this selector at the given offset within the given region. @@ -540,6 +540,8 @@ pub trait Circuit { pub enum Expression { /// This is a constant polynomial Constant(F), + /// This is a virtual selector + Selector(Selector), /// This is a fixed column queried at a certain relative location Fixed { /// Query index @@ -581,6 +583,7 @@ impl Expression { pub fn evaluate( &self, constant: &impl Fn(F) -> T, + selector_column: &impl Fn(Selector) -> T, fixed_column: &impl Fn(usize, usize, Rotation) -> T, advice_column: &impl Fn(usize, usize, Rotation) -> T, instance_column: &impl Fn(usize, usize, Rotation) -> T, @@ -590,6 +593,7 @@ impl Expression { ) -> T { match self { Expression::Constant(scalar) => constant(*scalar), + Expression::Selector(selector) => selector_column(*selector), Expression::Fixed { query_index, column_index, @@ -608,6 +612,7 @@ impl Expression { Expression::Sum(a, b) => { let a = a.evaluate( constant, + selector_column, fixed_column, advice_column, instance_column, @@ -617,6 +622,7 @@ impl Expression { ); let b = b.evaluate( constant, + selector_column, fixed_column, advice_column, instance_column, @@ -629,6 +635,7 @@ impl Expression { Expression::Product(a, b) => { let a = a.evaluate( constant, + selector_column, fixed_column, advice_column, instance_column, @@ -638,6 +645,7 @@ impl Expression { ); let b = b.evaluate( constant, + selector_column, fixed_column, advice_column, instance_column, @@ -650,6 +658,7 @@ impl Expression { Expression::Scaled(a, f) => { let a = a.evaluate( constant, + selector_column, fixed_column, advice_column, instance_column, @@ -666,6 +675,7 @@ impl Expression { pub fn degree(&self) -> usize { match self { Expression::Constant(_) => 0, + Expression::Selector(_) => 1, Expression::Fixed { .. } => 1, Expression::Advice { .. } => 1, Expression::Instance { .. } => 1, @@ -805,6 +815,8 @@ pub struct ConstraintSystem { pub(crate) num_fixed_columns: usize, pub(crate) num_advice_columns: usize, pub(crate) num_instance_columns: usize, + pub(crate) num_selectors: usize, + pub(crate) selector_map: Vec>, pub(crate) gates: Vec>, pub(crate) advice_queries: Vec<(Column, Rotation)>, // Contains an integer for each advice column @@ -834,6 +846,8 @@ pub struct PinnedConstraintSystem<'a, F: Field> { num_fixed_columns: &'a usize, num_advice_columns: &'a usize, num_instance_columns: &'a usize, + num_selectors: &'a usize, + selector_map: &'a [Column], gates: PinnedGates<'a, F>, advice_queries: &'a Vec<(Column, Rotation)>, instance_queries: &'a Vec<(Column, Rotation)>, @@ -860,6 +874,8 @@ impl Default for ConstraintSystem { num_fixed_columns: 0, num_advice_columns: 0, num_instance_columns: 0, + num_selectors: 0, + selector_map: vec![], gates: vec![], fixed_queries: Vec::new(), advice_queries: Vec::new(), @@ -882,6 +898,8 @@ impl ConstraintSystem { num_fixed_columns: &self.num_fixed_columns, num_advice_columns: &self.num_advice_columns, num_instance_columns: &self.num_instance_columns, + num_selectors: &self.num_selectors, + selector_map: &self.selector_map, gates: PinnedGates(&self.gates), fixed_queries: &self.fixed_queries, advice_queries: &self.advice_queries, @@ -1072,11 +1090,80 @@ impl ConstraintSystem { }); } + /// This will compress selectors together depending on their provided + /// assignments. This `ConstraintSystem` will then be modified to add new + /// fixed columns (representing the actual selectors) and will return the + /// polynomials for those columns. Finally, an internal map is updated to + /// find which fixed column corresponds with a given `Selector`. + /// + /// Do not call this twice. Yes, this should be a builder pattern instead. + pub(crate) fn compress_selectors(mut self, selectors: Vec>) -> (Self, Vec>) + where + F: FieldExt, + { + // Note: This is a stub for an actual optimization step. For now, just + // make new columns like we were doing already. + assert_eq!(selectors.len(), self.num_selectors); + + // Create self.num_selectors new fixed columns + let mut queries = vec![]; + for _ in 0..self.num_selectors { + let column = self.fixed_column(); + // Selectors are only queried here. + let query = self.query_fixed_index(column, Rotation::cur()); + queries.push(query); + self.selector_map.push(column); + } + + let polys = selectors + .into_iter() + .map(|bools| { + bools + .into_iter() + .map(|coeff| if coeff { F::one() } else { F::zero() }) + .collect() + }) + .collect(); + + // Substitute selectors for the real fixed columns in all gates + for gate in self.gates.iter_mut().flat_map(|gate| gate.polys.iter_mut()) { + let selector_map = &self.selector_map; + *gate = gate.evaluate( + &|constant| Expression::Constant(constant), + &|selector| Expression::Fixed { + query_index: queries[selector.0], + column_index: selector_map[selector.0].index(), + rotation: Rotation::cur(), + }, + &|query_index, column_index, rotation| Expression::Fixed { + query_index, + column_index, + rotation, + }, + &|query_index, column_index, rotation| Expression::Advice { + query_index, + column_index, + rotation, + }, + &|query_index, column_index, rotation| Expression::Instance { + query_index, + column_index, + rotation, + }, + &|a, b| a + b, + &|a, b| a * b, + &|a, f| a * f, + ); + } + + (self, polys) + } + /// Allocate a new selector. pub fn selector(&mut self) -> Selector { - // TODO: Track selectors separately, and combine selectors where possible. - // https://github.com/zcash/halo2/issues/116 - Selector(self.fixed_column()) + let index = self.num_selectors; + self.num_selectors += 1; + Selector(index) } /// Allocate a new fixed column @@ -1204,13 +1291,8 @@ impl<'a, F: Field> VirtualCells<'a, F> { /// Query a selector at the current position. pub fn query_selector(&mut self, selector: Selector) -> Expression { - // Selectors are always queried at the current row. self.queried_selectors.push(selector); - Expression::Fixed { - query_index: self.meta.query_fixed_index(selector.0, Rotation::cur()), - column_index: (selector.0).index, - rotation: Rotation::cur(), - } + Expression::Selector(selector) } /// Query a fixed column at a relative position diff --git a/src/plonk/keygen.rs b/src/plonk/keygen.rs index a1c7be23..3b32b83a 100644 --- a/src/plonk/keygen.rs +++ b/src/plonk/keygen.rs @@ -44,6 +44,7 @@ where struct Assembly { fixed: Vec, LagrangeCoeff>>, permutation: permutation::keygen::Assembly, + selectors: Vec>, // A range of available rows for assignment and copies. usable_rows: RangeTo, _marker: std::marker::PhantomData, @@ -62,12 +63,7 @@ impl Assignment for Assembly { // Do nothing; we don't care about regions in this context. } - fn enable_selector( - &mut self, - annotation: A, - selector: &Selector, - row: usize, - ) -> Result<(), Error> + fn enable_selector(&mut self, _: A, selector: &Selector, row: usize) -> Result<(), Error> where A: FnOnce() -> AR, AR: Into, @@ -75,12 +71,10 @@ impl Assignment for Assembly { if !self.usable_rows.contains(&row) { return Err(Error::BoundsFailure); } - // Selectors are just fixed columns. - // TODO: Ensure that the default for a selector's cells is always zero, if we - // alter the proving system to change the global default. - // TODO: Implement selector combining optimization - // https://github.com/zcash/halo2/issues/116 - self.assign_fixed(annotation, selector.0, row, || Ok(F::one())) + + self.selectors[selector.0][row] = true; + + Ok(()) } fn query_instance(&self, _: Column, row: usize) -> Result, Error> { @@ -181,6 +175,7 @@ where let mut assembly: Assembly = Assembly { fixed: vec![domain.empty_lagrange_assigned(); cs.num_fixed_columns], permutation: permutation::keygen::Assembly::new(params.n as usize, &cs.permutation), + selectors: vec![vec![false; params.n as usize]; cs.num_selectors], usable_rows: ..params.n as usize - (cs.blinding_factors() + 1), _marker: std::marker::PhantomData, }; @@ -193,7 +188,13 @@ where cs.constants.clone(), )?; - let fixed = batch_invert_assigned(assembly.fixed); + let mut fixed = batch_invert_assigned(assembly.fixed); + let (cs, selector_polys) = cs.compress_selectors(assembly.selectors); + fixed.extend( + selector_polys + .into_iter() + .map(|poly| domain.lagrange_from_vec(poly)), + ); let permutation_vk = assembly .permutation @@ -234,6 +235,7 @@ where let mut assembly: Assembly = Assembly { fixed: vec![vk.domain.empty_lagrange_assigned(); vk.cs.num_fixed_columns], permutation: permutation::keygen::Assembly::new(params.n as usize, &vk.cs.permutation), + selectors: vec![vec![false; params.n as usize]; cs.num_selectors], usable_rows: ..params.n as usize - (vk.cs.blinding_factors() + 1), _marker: std::marker::PhantomData, }; @@ -246,7 +248,13 @@ where cs.constants.clone(), )?; - let fixed = batch_invert_assigned(assembly.fixed); + let mut fixed = batch_invert_assigned(assembly.fixed); + let (cs, selector_polys) = cs.compress_selectors(assembly.selectors); + fixed.extend( + selector_polys + .into_iter() + .map(|poly| vk.domain.lagrange_from_vec(poly)), + ); let fixed_polys: Vec<_> = fixed .iter() diff --git a/src/plonk/lookup/prover.rs b/src/plonk/lookup/prover.rs index ae5b7bcb..3377c43f 100644 --- a/src/plonk/lookup/prover.rs +++ b/src/plonk/lookup/prover.rs @@ -101,6 +101,7 @@ impl Argument { .map(|expression| { expression.evaluate( &|scalar| pk.vk.domain.constant_lagrange(scalar), + &|_| panic!("virtual selectors are removed during optimization"), &|_, column_index, rotation| { fixed_values[column_index].clone().rotate(rotation) }, @@ -134,6 +135,7 @@ impl Argument { .map(|expression| { expression.evaluate( &|scalar| pk.vk.domain.constant_extended(scalar), + &|_| panic!("virtual selectors are removed during optimization"), &|_, column_index, rotation| { pk.vk .domain diff --git a/src/plonk/lookup/verifier.rs b/src/plonk/lookup/verifier.rs index 9e8db772..70c10b7f 100644 --- a/src/plonk/lookup/verifier.rs +++ b/src/plonk/lookup/verifier.rs @@ -134,6 +134,7 @@ impl Evaluated { .map(|expression| { expression.evaluate( &|scalar| scalar, + &|_| panic!("virtual selectors are removed during optimization"), &|index, _, _| fixed_evals[index], &|index, _, _| advice_evals[index], &|index, _, _| instance_evals[index], diff --git a/src/plonk/prover.rs b/src/plonk/prover.rs index 90b633d9..de796058 100644 --- a/src/plonk/prover.rs +++ b/src/plonk/prover.rs @@ -56,6 +56,10 @@ pub fn create_proof< let mut meta = ConstraintSystem::default(); let config = ConcreteCircuit::configure(&mut meta); + // Selector optimizations cannot be applied here; use the ConstraintSystem + // from the verification key. + let meta = &pk.vk.cs; + struct InstanceSingle { pub instance_values: Vec>, pub instance_polys: Vec>, @@ -431,6 +435,7 @@ pub fn create_proof< gate.polynomials().iter().map(move |poly| { poly.evaluate( &|scalar| pk.vk.domain.constant_extended(scalar), + &|_| panic!("virtual selectors are removed during optimization"), &|_, column_index, rotation| { pk.vk .domain diff --git a/src/plonk/verifier.rs b/src/plonk/verifier.rs index 3ee85f01..3a3fe006 100644 --- a/src/plonk/verifier.rs +++ b/src/plonk/verifier.rs @@ -184,6 +184,7 @@ pub fn verify_proof<'params, C: CurveAffine, E: EncodedChallenge, T: Transcri gate.polynomials().iter().map(move |poly| { poly.evaluate( &|scalar| scalar, + &|_| panic!("virtual selectors are removed during optimization"), &|index, _, _| fixed_evals[index], &|index, _, _| advice_evals[index], &|index, _, _| instance_evals[index], diff --git a/tests/plonk_api.rs b/tests/plonk_api.rs index 7a3625fd..2181960a 100644 --- a/tests/plonk_api.rs +++ b/tests/plonk_api.rs @@ -521,6 +521,8 @@ fn plonk_api() { num_fixed_columns: 8, num_advice_columns: 5, num_instance_columns: 1, + num_selectors: 0, + selector_map: [], gates: [ Sum( Sum( From 23d7d6532dfa4324721bdd743cd85e4db9c57e52 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 21 Jul 2021 13:12:05 -0600 Subject: [PATCH 03/19] Properly substitute selectors in lookup expressions during optimization pass. --- src/plonk/circuit.rs | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/plonk/circuit.rs b/src/plonk/circuit.rs index ffdba644..dbc84d35 100644 --- a/src/plonk/circuit.rs +++ b/src/plonk/circuit.rs @@ -1125,13 +1125,15 @@ impl ConstraintSystem { }) .collect(); - // Substitute selectors for the real fixed columns in all gates - for gate in self.gates.iter_mut().flat_map(|gate| gate.polys.iter_mut()) { - let selector_map = &self.selector_map; - *gate = gate.evaluate( + fn replace_selectors( + expr: &mut Expression, + selector_map: &[Column], + selector_queries: &[usize], + ) { + *expr = expr.evaluate( &|constant| Expression::Constant(constant), &|selector| Expression::Fixed { - query_index: queries[selector.0], + query_index: selector_queries[selector.0], column_index: selector_map[selector.0].index(), rotation: Rotation::cur(), }, @@ -1156,6 +1158,22 @@ impl ConstraintSystem { ); } + // Substitute selectors for the real fixed columns in all gates + for expr in self.gates.iter_mut().flat_map(|gate| gate.polys.iter_mut()) { + replace_selectors(expr, &self.selector_map, &queries); + } + + // Substitute selectors for the real fixed columns in all lookup + // expressions + for expr in self.lookups.iter_mut().flat_map(|lookup| { + lookup + .input_expressions + .iter_mut() + .chain(lookup.table_expressions.iter_mut()) + }) { + replace_selectors(expr, &self.selector_map, &queries); + } + (self, polys) } From 2cc60838a5e7ec10c65610af96cb262351e715f9 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 21 Jul 2021 13:34:56 -0600 Subject: [PATCH 04/19] Use locally generated ConstraintSystem to avoid performing optimizations twice. --- src/plonk/keygen.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/plonk/keygen.rs b/src/plonk/keygen.rs index 3b32b83a..68d3835c 100644 --- a/src/plonk/keygen.rs +++ b/src/plonk/keygen.rs @@ -233,10 +233,10 @@ where } let mut assembly: Assembly = Assembly { - fixed: vec![vk.domain.empty_lagrange_assigned(); vk.cs.num_fixed_columns], - permutation: permutation::keygen::Assembly::new(params.n as usize, &vk.cs.permutation), + fixed: vec![vk.domain.empty_lagrange_assigned(); cs.num_fixed_columns], + permutation: permutation::keygen::Assembly::new(params.n as usize, &cs.permutation), selectors: vec![vec![false; params.n as usize]; cs.num_selectors], - usable_rows: ..params.n as usize - (vk.cs.blinding_factors() + 1), + usable_rows: ..params.n as usize - (cs.blinding_factors() + 1), _marker: std::marker::PhantomData, }; @@ -268,7 +268,7 @@ where let permutation_pk = assembly .permutation - .build_pk(params, &vk.domain, &vk.cs.permutation); + .build_pk(params, &vk.domain, &cs.permutation); // Compute l_0(X) // TODO: this can be done more efficiently From f6895555bb3368af5a9859e459fe17bc6a66b15a Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 21 Jul 2021 13:51:48 -0600 Subject: [PATCH 05/19] Update cost model logic to account for selector optimizations. --- src/dev/cost.rs | 107 +++++++++++++++++++++++++++++++++++++++++++-- tests/plonk_api.rs | 2 +- 2 files changed, 104 insertions(+), 5 deletions(-) diff --git a/src/dev/cost.rs b/src/dev/cost.rs index f989ae9f..ff899f19 100644 --- a/src/dev/cost.rs +++ b/src/dev/cost.rs @@ -6,11 +6,15 @@ use std::{ marker::PhantomData, }; -use ff::PrimeField; +use ff::{Field, PrimeField}; use group::prime::PrimeGroup; use crate::{ - plonk::{Any, Circuit, Column, ConstraintSystem}, + arithmetic::FieldExt, + plonk::{ + Advice, Any, Assigned, Assignment, Circuit, Column, ConstraintSystem, Error, Fixed, + FloorPlanner, Instance, Selector, + }, poly::Rotation, }; @@ -37,14 +41,109 @@ pub struct CircuitCost> { _marker: PhantomData<(G, ConcreteCircuit)>, } +struct Assembly { + selectors: Vec>, +} + +impl Assignment for Assembly { + fn enter_region(&mut self, _: N) + where + NR: Into, + N: FnOnce() -> NR, + { + // Do nothing; we don't care about regions in this context. + } + + fn exit_region(&mut self) { + // Do nothing; we don't care about regions in this context. + } + + fn enable_selector(&mut self, _: A, selector: &Selector, row: usize) -> Result<(), Error> + where + A: FnOnce() -> AR, + AR: Into, + { + self.selectors[selector.0][row] = true; + + Ok(()) + } + + fn query_instance(&self, _: Column, _: usize) -> Result, Error> { + Ok(None) + } + + fn assign_advice( + &mut self, + _: A, + _: Column, + _: usize, + _: V, + ) -> Result<(), Error> + where + V: FnOnce() -> Result, + VR: Into>, + A: FnOnce() -> AR, + AR: Into, + { + Ok(()) + } + + fn assign_fixed( + &mut self, + _: A, + _: Column, + _: usize, + _: V, + ) -> Result<(), Error> + where + V: FnOnce() -> Result, + VR: Into>, + A: FnOnce() -> AR, + AR: Into, + { + Ok(()) + } + + fn copy(&mut self, _: Column, _: usize, _: Column, _: usize) -> Result<(), Error> { + Ok(()) + } + + fn push_namespace(&mut self, _: N) + where + NR: Into, + N: FnOnce() -> NR, + { + // Do nothing; we don't care about namespaces in this context. + } + + fn pop_namespace(&mut self, _: Option) { + // Do nothing; we don't care about namespaces in this context. + } +} + impl> CircuitCost { /// Measures a circuit with parameter constant `k`. /// /// Panics if `k` is not large enough for the circuit. - pub fn measure(k: usize) -> Self { + pub fn measure(k: usize, circuit: &ConcreteCircuit) -> Self + where + G::Scalar: FieldExt, + { // Collect the layout details. let mut cs = ConstraintSystem::default(); - let _ = ConcreteCircuit::configure(&mut cs); + let config = ConcreteCircuit::configure(&mut cs); + let mut assembly = Assembly { + selectors: vec![vec![false; 1 << k]; cs.num_selectors], + }; + ConcreteCircuit::FloorPlanner::synthesize( + &mut assembly, + circuit, + config, + cs.constants.clone(), + ) + .unwrap(); + let (cs, _) = cs.compress_selectors(assembly.selectors); + assert!((1 << k) >= cs.minimum_rows()); // Figure out how many point sets we have due to queried cells. diff --git a/tests/plonk_api.rs b/tests/plonk_api.rs index 2181960a..8beac2b5 100644 --- a/tests/plonk_api.rs +++ b/tests/plonk_api.rs @@ -454,7 +454,7 @@ fn plonk_api() { let proof: Vec = transcript.finalize(); assert_eq!( proof.len(), - halo2::dev::CircuitCost::>::measure(K as usize) + halo2::dev::CircuitCost::>::measure(K as usize, &circuit) .proof_size(2) .into(), ); From 1254e4f245dc541eeff77fecb9af2a34b18e9419 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 21 Jul 2021 14:43:31 -0600 Subject: [PATCH 06/19] Ensure that selectors are only used in simple ways. You cannot add an expression containing a Selector to another expression. You cannot multiply an expression containing a Selector by another expression that also has a selector. --- src/plonk/circuit.rs | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/src/plonk/circuit.rs b/src/plonk/circuit.rs index dbc84d35..9bf649f1 100644 --- a/src/plonk/circuit.rs +++ b/src/plonk/circuit.rs @@ -689,6 +689,20 @@ impl Expression { pub fn square(self) -> Self { self.clone() * self } + + /// Returns whether or not this expression contains a `Selector`. + fn contains_selector(&self) -> bool { + self.evaluate( + &|_| false, + &|_| true, + &|_, _, _| false, + &|_, _, _| false, + &|_, _, _| false, + &|a, b| a || b, + &|a, b| a || b, + &|a, _| a, + ) + } } impl Neg for Expression { @@ -698,9 +712,12 @@ impl Neg for Expression { } } -impl Add for Expression { +impl Add for Expression { type Output = Expression; fn add(self, rhs: Expression) -> Expression { + if self.contains_selector() || rhs.contains_selector() { + panic!("attempted to add to a selector"); + } Expression::Sum(Box::new(self), Box::new(rhs)) } } @@ -708,18 +725,24 @@ impl Add for Expression { impl Sub for Expression { type Output = Expression; fn sub(self, rhs: Expression) -> Expression { + if self.contains_selector() || rhs.contains_selector() { + panic!("attempted to add to a selector"); + } Expression::Sum(Box::new(self), Box::new(-rhs)) } } -impl Mul for Expression { +impl Mul for Expression { type Output = Expression; fn mul(self, rhs: Expression) -> Expression { + if self.contains_selector() && rhs.contains_selector() { + panic!("attempted to multiply two selectors"); + } Expression::Product(Box::new(self), Box::new(rhs)) } } -impl Mul for Expression { +impl Mul for Expression { type Output = Expression; fn mul(self, rhs: F) -> Expression { Expression::Scaled(Box::new(self), rhs) From 26982a6050e9e95f3f46faf7286c263f38f48f55 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 21 Jul 2021 15:45:36 -0600 Subject: [PATCH 07/19] Update circuit graph tooling to account for selector optimization. --- examples/circuit-layout.rs | 4 +- src/dev/graph/layout.rs | 85 +++++++++++++++++++++++--------------- src/plonk/circuit.rs | 11 ++--- 3 files changed, 59 insertions(+), 41 deletions(-) diff --git a/examples/circuit-layout.rs b/examples/circuit-layout.rs index 155f23b7..02eb1063 100644 --- a/examples/circuit-layout.rs +++ b/examples/circuit-layout.rs @@ -350,5 +350,7 @@ fn main() { .titled("Example Circuit Layout", ("sans-serif", 60)) .unwrap(); - CircuitLayout::default().render(&circuit, &root).unwrap(); + CircuitLayout::default() + .render(1 << 5, &circuit, &root) + .unwrap(); } diff --git a/src/dev/graph/layout.rs b/src/dev/graph/layout.rs index 814b7a1b..e8fe228e 100644 --- a/src/dev/graph/layout.rs +++ b/src/dev/graph/layout.rs @@ -7,6 +7,7 @@ use std::cmp; use std::collections::HashSet; use std::ops::Range; +use crate::circuit::layouter::RegionColumn; use crate::plonk::{ Advice, Any, Assigned, Assignment, Circuit, Column, ConstraintSystem, Error, Fixed, FloorPlanner, Instance, Selector, @@ -83,16 +84,18 @@ impl CircuitLayout { /// Renders the given circuit on the given drawing area. pub fn render, DB: DrawingBackend>( self, + k: usize, circuit: &ConcreteCircuit, drawing_area: &DrawingArea, ) -> Result<(), DrawingAreaErrorKind> { use plotters::coord::types::RangedCoordusize; use plotters::prelude::*; + let n = 1 << k; // Collect the layout details. let mut cs = ConstraintSystem::default(); let config = ConcreteCircuit::configure(&mut cs); - let mut layout = Layout::default(); + let mut layout = Layout::new(n, cs.num_selectors); ConcreteCircuit::FloorPlanner::synthesize( &mut layout, circuit, @@ -100,11 +103,17 @@ impl CircuitLayout { cs.constants.clone(), ) .unwrap(); + let (cs, selector_polys) = cs.compress_selectors(layout.selectors); + let non_selector_fixed_columns = cs.num_fixed_columns - selector_polys.len(); // Figure out what order to render the columns in. // TODO: For now, just render them in the order they were configured. let total_columns = cs.num_instance_columns + cs.num_advice_columns + cs.num_fixed_columns; - let column_index = |column: &Column| { + let column_index = |cs: &ConstraintSystem, column: RegionColumn| { + let column: Column = match column { + RegionColumn::Column(col) => col, + RegionColumn::Selector(selector) => cs.selector_map[selector.0].into(), + }; column.index() + match column.column_type() { Any::Instance => 0, @@ -143,15 +152,16 @@ impl CircuitLayout { ], ShapeStyle::from(&BLUE.mix(0.2)).filled(), ))?; - for selector in layout.selectors { - let index = selector.index(); + { root.draw(&Rectangle::new( [ - (cs.num_instance_columns + cs.num_advice_columns + index, 0), ( - cs.num_instance_columns + cs.num_advice_columns + index + 1, - view_bottom, + cs.num_instance_columns + + cs.num_advice_columns + + non_selector_fixed_columns, + 0, ), + (total_columns, view_bottom), ], ShapeStyle::from(&BLUE.mix(0.1)).filled(), ))?; @@ -191,12 +201,12 @@ impl CircuitLayout { if let Some(offset) = region.offset { // Sort the region's columns according to the defined ordering. let mut columns: Vec<_> = region.columns.into_iter().collect(); - columns.sort_unstable_by_key(|a| column_index(a)); + columns.sort_unstable_by_key(|a| column_index(&cs, *a)); // Render contiguous parts of the same region as a single box. let mut width = None; for column in columns { - let column = column_index(&column); + let column = column_index(&cs, column); match width { Some((start, end)) if end == column => width = Some((start, end + 1)), Some((start, end)) => { @@ -220,22 +230,22 @@ impl CircuitLayout { // Darken the cells of the region that have been assigned to. for (column, row) in region.cells { - draw_cell(&root, column_index(&column), row)?; + draw_cell(&root, column_index(&cs, column), row)?; } } } // Darken any loose cells that have been assigned to. for (column, row) in layout.loose_cells { - draw_cell(&root, column_index(&column), row)?; + draw_cell(&root, column_index(&cs, column), row)?; } // Mark equality-constrained cells. if self.mark_equality_cells { let mut cells = HashSet::new(); for (l_col, l_row, r_col, r_row) in &layout.equality { - let l_col = column_index(l_col); - let r_col = column_index(r_col); + let l_col = column_index(&cs, (*l_col).into()); + let r_col = column_index(&cs, (*r_col).into()); // Deduplicate cells. cells.insert((l_col, *l_row)); @@ -253,8 +263,8 @@ impl CircuitLayout { // Draw lines between equality-constrained cells. if self.show_equality_constraints { for (l_col, l_row, r_col, r_row) in &layout.equality { - let l_col = column_index(l_col); - let r_col = column_index(r_col); + let l_col = column_index(&cs, (*l_col).into()); + let r_col = column_index(&cs, (*r_col).into()); root.draw(&PathElement::new( [(l_col, *l_row), (r_col, *r_row)], ShapeStyle::from(&RED), @@ -280,14 +290,14 @@ struct Region { /// The name of the region. Not required to be unique. name: String, /// The columns used by this region. - columns: HashSet>, + columns: HashSet, /// The row that this region starts on, if known. offset: Option, /// The number of rows that this region takes up. rows: usize, /// The cells assigned in this region. We store this as a `Vec` so that if any cells /// are double-assigned, they will be visibly darker. - cells: Vec<(Column, usize)>, + cells: Vec<(RegionColumn, usize)>, } #[derive(Default)] @@ -297,15 +307,30 @@ struct Layout { total_rows: usize, /// Any cells assigned outside of a region. We store this as a `Vec` so that if any /// cells are double-assigned, they will be visibly darker. - loose_cells: Vec<(Column, usize)>, - /// Columns we have observed are actually Selectors. - selectors: HashSet>, + loose_cells: Vec<(RegionColumn, usize)>, /// Pairs of cells between which we have equality constraints. equality: Vec<(Column, usize, Column, usize)>, + /// Selector assignments used for optimization pass + selectors: Vec>, } impl Layout { - fn update(&mut self, column: Column, row: usize) { + fn new(n: usize, num_selectors: usize) -> Self { + Layout { + regions: vec![], + current_region: None, + total_rows: 0, + /// Any cells assigned outside of a region. We store this as a `Vec` so that if any + /// cells are double-assigned, they will be visibly darker. + loose_cells: vec![], + /// Pairs of cells between which we have equality constraints. + equality: vec![], + /// Selector assignments used for optimization pass + selectors: vec![vec![false; n]; num_selectors], + } + } + + fn update(&mut self, column: RegionColumn, row: usize) { self.total_rows = cmp::max(self.total_rows, row + 1); if let Some(region) = self.current_region { @@ -353,20 +378,14 @@ impl Assignment for Layout { self.current_region = None; } - fn enable_selector( - &mut self, - annotation: A, - selector: &Selector, - row: usize, - ) -> Result<(), Error> + fn enable_selector(&mut self, _: A, selector: &Selector, row: usize) -> Result<(), Error> where A: FnOnce() -> AR, AR: Into, { - // Remember that this column is a selector. - self.selectors.insert(selector.0.into()); - // Selectors are just fixed columns. - self.assign_fixed(annotation, selector.0, row, || Ok(F::one())) + self.selectors[selector.0][row] = true; + self.update((*selector).into(), row); + Ok(()) } fn query_instance(&self, _: Column, _: usize) -> Result, Error> { @@ -386,7 +405,7 @@ impl Assignment for Layout { A: FnOnce() -> AR, AR: Into, { - self.update(column.into(), row); + self.update(Column::::from(column).into(), row); Ok(()) } @@ -403,7 +422,7 @@ impl Assignment for Layout { A: FnOnce() -> AR, AR: Into, { - self.update(column.into(), row); + self.update(Column::::from(column).into(), row); Ok(()) } diff --git a/src/plonk/circuit.rs b/src/plonk/circuit.rs index 9bf649f1..c35bfe4d 100644 --- a/src/plonk/circuit.rs +++ b/src/plonk/circuit.rs @@ -8,7 +8,7 @@ use std::{ use super::{lookup, permutation, Error}; use crate::circuit::Layouter; -use crate::{arithmetic::FieldExt, circuit::Region, poly::Rotation}; +use crate::{circuit::Region, poly::Rotation}; /// A column type pub trait ColumnType: @@ -232,7 +232,7 @@ pub struct Selector(pub(crate) usize); impl Selector { /// Enable this selector at the given offset within the given region. - pub fn enable(&self, region: &mut Region, offset: usize) -> Result<(), Error> { + pub fn enable(&self, region: &mut Region, offset: usize) -> Result<(), Error> { region.enable_selector(|| "", self, offset) } } @@ -1120,10 +1120,7 @@ impl ConstraintSystem { /// find which fixed column corresponds with a given `Selector`. /// /// Do not call this twice. Yes, this should be a builder pattern instead. - pub(crate) fn compress_selectors(mut self, selectors: Vec>) -> (Self, Vec>) - where - F: FieldExt, - { + pub(crate) fn compress_selectors(mut self, selectors: Vec>) -> (Self, Vec>) { // Note: This is a stub for an actual optimization step. For now, just // make new columns like we were doing already. assert_eq!(selectors.len(), self.num_selectors); @@ -1148,7 +1145,7 @@ impl ConstraintSystem { }) .collect(); - fn replace_selectors( + fn replace_selectors( expr: &mut Expression, selector_map: &[Column], selector_queries: &[usize], From 145d096dcd6f840178e97ec2bfd6ca9c6201093e Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Thu, 22 Jul 2021 10:07:20 -0600 Subject: [PATCH 08/19] Distinguish between simple and non-simple selectors, and allow the former in lookup arguments. --- src/plonk/circuit.rs | 71 ++++++++++++++++++++++++++---------- src/plonk/lookup/prover.rs | 2 +- src/plonk/lookup/verifier.rs | 2 +- 3 files changed, 54 insertions(+), 21 deletions(-) diff --git a/src/plonk/circuit.rs b/src/plonk/circuit.rs index c35bfe4d..a0e88807 100644 --- a/src/plonk/circuit.rs +++ b/src/plonk/circuit.rs @@ -228,13 +228,19 @@ impl TryFrom> for Column { /// } /// ``` #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] -pub struct Selector(pub(crate) usize); +pub struct Selector(pub(crate) usize, bool); impl Selector { /// Enable this selector at the given offset within the given region. pub fn enable(&self, region: &mut Region, offset: usize) -> Result<(), Error> { region.enable_selector(|| "", self, offset) } + + /// Is this selector "simple"? Simple selectors can only be multiplied + /// by expressions that contain no other simple selectors. + pub fn is_simple(&self) -> bool { + self.1 + } } /// A value assigned to a cell within a circuit. @@ -691,10 +697,10 @@ impl Expression { } /// Returns whether or not this expression contains a `Selector`. - fn contains_selector(&self) -> bool { + fn contains_simple_selector(&self) -> bool { self.evaluate( &|_| false, - &|_| true, + &|selector| selector.is_simple(), &|_, _, _| false, &|_, _, _| false, &|_, _, _| false, @@ -715,8 +721,8 @@ impl Neg for Expression { impl Add for Expression { type Output = Expression; fn add(self, rhs: Expression) -> Expression { - if self.contains_selector() || rhs.contains_selector() { - panic!("attempted to add to a selector"); + if self.contains_simple_selector() || rhs.contains_simple_selector() { + panic!("attempted to add to a simple selector"); } Expression::Sum(Box::new(self), Box::new(rhs)) } @@ -725,8 +731,8 @@ impl Add for Expression { impl Sub for Expression { type Output = Expression; fn sub(self, rhs: Expression) -> Expression { - if self.contains_selector() || rhs.contains_selector() { - panic!("attempted to add to a selector"); + if self.contains_simple_selector() || rhs.contains_simple_selector() { + panic!("attempted to add to a simple selector"); } Expression::Sum(Box::new(self), Box::new(-rhs)) } @@ -735,8 +741,8 @@ impl Sub for Expression { impl Mul for Expression { type Output = Expression; fn mul(self, rhs: Expression) -> Expression { - if self.contains_selector() && rhs.contains_selector() { - panic!("attempted to multiply two selectors"); + if self.contains_simple_selector() && rhs.contains_simple_selector() { + panic!("attempted to multiply two simple selectors"); } Expression::Product(Box::new(self), Box::new(rhs)) } @@ -963,6 +969,12 @@ impl ConstraintSystem { let mut cells = VirtualCells::new(self); let table_map = table_map(&mut cells); + for table in &table_map { + if table.0.contains_simple_selector() || table.1.contains_simple_selector() { + panic!("expression containing simple selector supplied to lookup argument"); + } + } + let index = self.lookups.len(); self.lookups.push(lookup::Argument::new(table_map)); @@ -1149,13 +1161,23 @@ impl ConstraintSystem { expr: &mut Expression, selector_map: &[Column], selector_queries: &[usize], + must_be_nonsimple: bool, ) { *expr = expr.evaluate( &|constant| Expression::Constant(constant), - &|selector| Expression::Fixed { - query_index: selector_queries[selector.0], - column_index: selector_map[selector.0].index(), - rotation: Rotation::cur(), + &|selector| { + if must_be_nonsimple { + // Complex selectors are prohibited from appearing in + // expressions in the lookup argument by + // `ConstraintSystem`. + assert!(!selector.is_simple()); + } + + Expression::Fixed { + query_index: selector_queries[selector.0], + column_index: selector_map[selector.0].index(), + rotation: Rotation::cur(), + } }, &|query_index, column_index, rotation| Expression::Fixed { query_index, @@ -1180,28 +1202,39 @@ impl ConstraintSystem { // Substitute selectors for the real fixed columns in all gates for expr in self.gates.iter_mut().flat_map(|gate| gate.polys.iter_mut()) { - replace_selectors(expr, &self.selector_map, &queries); + replace_selectors(expr, &self.selector_map, &queries, false); } - // Substitute selectors for the real fixed columns in all lookup - // expressions + // Substitute non-simple selectors for the real fixed columns in all + // lookup expressions for expr in self.lookups.iter_mut().flat_map(|lookup| { lookup .input_expressions .iter_mut() .chain(lookup.table_expressions.iter_mut()) }) { - replace_selectors(expr, &self.selector_map, &queries); + replace_selectors(expr, &self.selector_map, &queries, true); } (self, polys) } - /// Allocate a new selector. + /// Allocate a new (simple) selector. Simple selectors cannot be added to + /// expressions nor multiplied by other expressions containing simple + /// selectors. Also, simple selectors may not appear in lookup argument + /// inputs. pub fn selector(&mut self) -> Selector { let index = self.num_selectors; self.num_selectors += 1; - Selector(index) + Selector(index, true) + } + + /// Allocate a new complex selector that can appear anywhere + /// within expressions. + pub fn complex_selector(&mut self) -> Selector { + let index = self.num_selectors; + self.num_selectors += 1; + Selector(index, false) } /// Allocate a new fixed column diff --git a/src/plonk/lookup/prover.rs b/src/plonk/lookup/prover.rs index 3377c43f..7c9dd40a 100644 --- a/src/plonk/lookup/prover.rs +++ b/src/plonk/lookup/prover.rs @@ -135,7 +135,7 @@ impl Argument { .map(|expression| { expression.evaluate( &|scalar| pk.vk.domain.constant_extended(scalar), - &|_| panic!("virtual selectors are removed during optimization"), + &|_| panic!("virtual selectors are optimized away during keygen"), &|_, column_index, rotation| { pk.vk .domain diff --git a/src/plonk/lookup/verifier.rs b/src/plonk/lookup/verifier.rs index 70c10b7f..e33e5066 100644 --- a/src/plonk/lookup/verifier.rs +++ b/src/plonk/lookup/verifier.rs @@ -134,7 +134,7 @@ impl Evaluated { .map(|expression| { expression.evaluate( &|scalar| scalar, - &|_| panic!("virtual selectors are removed during optimization"), + &|_| panic!("virtual selectors are optimized away during keygen"), &|index, _, _| fixed_evals[index], &|index, _, _| advice_evals[index], &|index, _, _| instance_evals[index], From 5af7a7abfa4a97a9dcdde8bc67dbdbc19bb7205f Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Thu, 22 Jul 2021 10:28:52 -0600 Subject: [PATCH 09/19] Only accept fixed columns in lookup argument tables. --- src/plonk/circuit.rs | 19 +++--- tests/plonk_api.rs | 138 +++++++++---------------------------------- 2 files changed, 41 insertions(+), 116 deletions(-) diff --git a/src/plonk/circuit.rs b/src/plonk/circuit.rs index a0e88807..190e7d3f 100644 --- a/src/plonk/circuit.rs +++ b/src/plonk/circuit.rs @@ -964,16 +964,21 @@ impl ConstraintSystem { /// they need to match. pub fn lookup( &mut self, - table_map: impl FnOnce(&mut VirtualCells<'_, F>) -> Vec<(Expression, Expression)>, + table_map: impl FnOnce(&mut VirtualCells<'_, F>) -> Vec<(Expression, Column)>, ) -> usize { let mut cells = VirtualCells::new(self); - let table_map = table_map(&mut cells); + let table_map = table_map(&mut cells) + .into_iter() + .map(|(input, table)| { + if input.contains_simple_selector() { + panic!("expression containing simple selector supplied to lookup argument"); + } - for table in &table_map { - if table.0.contains_simple_selector() || table.1.contains_simple_selector() { - panic!("expression containing simple selector supplied to lookup argument"); - } - } + let table = cells.query_fixed(table, Rotation::cur()); + + (input, table) + }) + .collect(); let index = self.lookups.len(); diff --git a/tests/plonk_api.rs b/tests/plonk_api.rs index 8beac2b5..dfa65cd9 100644 --- a/tests/plonk_api.rs +++ b/tests/plonk_api.rs @@ -38,7 +38,6 @@ fn plonk_api() { sm: Column, sp: Column, sl: Column, - sl2: Column, } trait StandardCs { @@ -63,14 +62,14 @@ fn plonk_api() { fn lookup_table( &self, layouter: &mut impl Layouter, - values: &[Vec], + values: &[FF], ) -> Result<(), Error>; } #[derive(Clone)] struct MyCircuit { a: Option, - lookup_tables: Vec>, + lookup_table: Vec, } struct StandardPlonk { @@ -227,26 +226,13 @@ fn plonk_api() { fn lookup_table( &self, layouter: &mut impl Layouter, - values: &[Vec], + values: &[FF], ) -> Result<(), Error> { layouter.assign_region( || "", |mut region| { - for (index, (&value_0, &value_1)) in - values[0].iter().zip(values[1].iter()).enumerate() - { - region.assign_fixed( - || "table col 1", - self.config.sl, - index, - || Ok(value_0), - )?; - region.assign_fixed( - || "table col 2", - self.config.sl2, - index, - || Ok(value_1), - )?; + for (index, &value) in values.iter().enumerate() { + region.assign_fixed(|| "table col", self.config.sl, index, || Ok(value))?; } Ok(()) }, @@ -262,7 +248,7 @@ fn plonk_api() { fn without_witnesses(&self) -> Self { Self { a: None, - lookup_tables: self.lookup_tables.clone(), + lookup_table: self.lookup_table.clone(), } } @@ -285,35 +271,26 @@ fn plonk_api() { let sc = meta.fixed_column(); let sp = meta.fixed_column(); let sl = meta.fixed_column(); - let sl2 = meta.fixed_column(); /* - * A B ... sl sl2 + * A B ... sl * [ - * instance 0 ... 0 0 - * a a ... 0 0 - * a a^2 ... 0 0 - * a a ... 0 0 - * a a^2 ... 0 0 - * ... ... ... ... ... - * ... ... ... instance 0 - * ... ... ... a a - * ... ... ... a a^2 - * ... ... ... 0 0 + * instance 0 ... 0 + * a a ... 0 + * a a^2 ... 0 + * a a ... 0 + * a a^2 ... 0 + * ... ... ... ... + * ... ... ... instance + * ... ... ... a + * ... ... ... a + * ... ... ... 0 * ] */ meta.lookup(|meta| { let a_ = meta.query_any(a.into(), Rotation::cur()); - let sl_ = meta.query_any(sl.into(), Rotation::cur()); - vec![(a_, sl_)] - }); - meta.lookup(|meta| { - let a_ = meta.query_any(a.into(), Rotation::cur()); - let b_ = meta.query_any(b.into(), Rotation::cur()); - let sl_ = meta.query_any(sl.into(), Rotation::cur()); - let sl2_ = meta.query_any(sl2.into(), Rotation::cur()); - vec![(a_ * b_, sl_ * sl2_)] + vec![(a_, sl)] }); meta.create_gate("Combined add-mult", |meta| { @@ -356,7 +333,6 @@ fn plonk_api() { meta.enable_equality(sc.into()); meta.enable_equality(sp.into()); meta.enable_equality(sl.into()); - meta.enable_equality(sl2.into()); PlonkConfig { a, @@ -370,7 +346,6 @@ fn plonk_api() { sm, sp, sl, - sl2, } } @@ -405,26 +380,24 @@ fn plonk_api() { cs.copy(&mut layouter, b1, c0)?; } - cs.lookup_table(&mut layouter, &self.lookup_tables)?; + cs.lookup_table(&mut layouter, &self.lookup_table)?; Ok(()) } } let a = Fp::from_u64(2834758237) * Fp::ZETA; - let a_squared = a * &a; let instance = Fp::one() + Fp::one(); let lookup_table = vec![instance, a, a, Fp::zero()]; - let lookup_table_2 = vec![Fp::zero(), a, a_squared, Fp::zero()]; let empty_circuit: MyCircuit = MyCircuit { a: None, - lookup_tables: vec![lookup_table.clone(), lookup_table_2.clone()], + lookup_table: lookup_table.clone(), }; let circuit: MyCircuit = MyCircuit { a: Some(a), - lookup_tables: vec![lookup_table, lookup_table_2], + lookup_table: lookup_table, }; // Initialize the proving key @@ -514,11 +487,11 @@ fn plonk_api() { scalar_modulus: "0x40000000000000000000000000000000224698fc094cf91b992d30ed00000001", domain: PinnedEvaluationDomain { k: 5, - extended_k: 8, + extended_k: 7, omega: 0x0cc3380dc616f2e1daf29ad1560833ed3baea3393eceb7bc8fa36376929b78cc, }, cs: PinnedConstraintSystem { - num_fixed_columns: 8, + num_fixed_columns: 7, num_advice_columns: 5, num_instance_columns: 1, num_selectors: 0, @@ -537,7 +510,7 @@ fn plonk_api() { ), }, Fixed { - query_index: 3, + query_index: 2, column_index: 2, rotation: Rotation( 0, @@ -553,7 +526,7 @@ fn plonk_api() { ), }, Fixed { - query_index: 4, + query_index: 3, column_index: 3, rotation: Rotation( 0, @@ -579,7 +552,7 @@ fn plonk_api() { }, ), Fixed { - query_index: 6, + query_index: 5, column_index: 1, rotation: Rotation( 0, @@ -597,7 +570,7 @@ fn plonk_api() { ), }, Fixed { - query_index: 5, + query_index: 4, column_index: 4, rotation: Rotation( 0, @@ -609,7 +582,7 @@ fn plonk_api() { ), Product( Fixed { - query_index: 2, + query_index: 1, column_index: 0, rotation: Rotation( 0, @@ -635,7 +608,7 @@ fn plonk_api() { ), Product( Fixed { - query_index: 7, + query_index: 6, column_index: 5, rotation: Rotation( 0, @@ -748,15 +721,6 @@ fn plonk_api() { 0, ), ), - ( - Column { - index: 7, - column_type: Fixed, - }, - Rotation( - 0, - ), - ), ( Column { index: 0, @@ -866,10 +830,6 @@ fn plonk_api() { index: 6, column_type: Fixed, }, - Column { - index: 7, - column_type: Fixed, - }, ], }, lookups: [ @@ -893,44 +853,6 @@ fn plonk_api() { }, ], }, - Argument { - input_expressions: [ - Product( - Advice { - query_index: 0, - column_index: 1, - rotation: Rotation( - 0, - ), - }, - Advice { - query_index: 1, - column_index: 2, - rotation: Rotation( - 0, - ), - }, - ), - ], - table_expressions: [ - Product( - Fixed { - query_index: 0, - column_index: 6, - rotation: Rotation( - 0, - ), - }, - Fixed { - query_index: 1, - column_index: 7, - rotation: Rotation( - 0, - ), - }, - ), - ], - }, ], constants: [], minimum_degree: None, @@ -943,7 +865,6 @@ fn plonk_api() { (0x02e62cd68370b13711139a08cbcdd889e800a272b9ea10acc90880fff9d89199, 0x1a96c468cb0ce77065d3a58f1e55fea9b72d15e44c01bba1e110bd0cbc6e9bc6), (0x224ef42758215157d3ee48fb8d769da5bddd35e5929a90a4a89736f5c4b5ae9b, 0x11bc3a1e08eb320cde764f1492ecef956d71e996e2165f7a9a30ad2febb511c1), (0x3c145eb1e4f1e49d9eed351a4e2d9f3deed13bc5ba028d3b425084d606418cc8, 0x045d846e7df4e563ce57cd5483d17bad87f0345e18409bf15abc3d71953ae71c), - (0x27b1cd6c0408a2fe7a764e6ac7abda4f6c7e7a4b3f7375532fe11f3af579de64, 0x19dcda088f6c8ad67408650554cfdd5c8c2e5385cf59c662554c837cf3f42c2d), ], permutation: VerifyingKey { commitments: [ @@ -960,7 +881,6 @@ fn plonk_api() { (0x394437571f9de32dccdc546fd4737772d8d92593c85438aa3473243997d5acc8, 0x14924ec6e3174f1fab7f0ce7070c22f04bbd0a0ecebdfc5c94be857f25493e95), (0x3d907e0591343bd285c2c846f3e871a6ac70d80ec29e9500b8cb57f544e60202, 0x1034e48df35830244cabea076be8a16d67d7896e27c6ac22b285d017105da9c3), (0x21d210b41675a1eae44cbd0f3fd27d69e30716c71873f6089cee61acacd403ab, 0x2275e97c7e84f68bfaa528a9d8be4e059f7abefd80d03fbfca774e8414a9b7c1), - (0x0f9e7de28e0f650d99d99d95c0fcd39c9dac9db5aa1973319f66922d6eb9f7d5, 0x1ba644ecc18ad711ddd33af7f695f6834e9f35c93d47a6a5273dabbe800fc7e6), ], }, }"##### From e3c6542e830c0eca7c054236311aa407c972d883 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Thu, 22 Jul 2021 12:04:41 -0600 Subject: [PATCH 10/19] Fix circuit-layout example. --- examples/circuit-layout.rs | 66 ++++++++++++-------------------------- 1 file changed, 20 insertions(+), 46 deletions(-) diff --git a/examples/circuit-layout.rs b/examples/circuit-layout.rs index 02eb1063..5593b44b 100644 --- a/examples/circuit-layout.rs +++ b/examples/circuit-layout.rs @@ -28,7 +28,6 @@ fn main() { sc: Column, sm: Column, sl: Column, - sl2: Column, } trait StandardCs { @@ -46,13 +45,13 @@ fn main() { fn lookup_table( &self, layouter: &mut impl Layouter, - values: &[Vec], + values: &[FF], ) -> Result<(), Error>; } struct MyCircuit { a: Option, - lookup_tables: Vec>, + lookup_table: Vec, } struct StandardPlonk { @@ -170,26 +169,13 @@ fn main() { fn lookup_table( &self, layouter: &mut impl Layouter, - values: &[Vec], + values: &[FF], ) -> Result<(), Error> { layouter.assign_region( || "", |mut region| { - for (index, (&value_0, &value_1)) in - values[0].iter().zip(values[1].iter()).enumerate() - { - region.assign_fixed( - || "table col 1", - self.config.sl, - index, - || Ok(value_0), - )?; - region.assign_fixed( - || "table col 2", - self.config.sl2, - index, - || Ok(value_1), - )?; + for (index, &value) in values.iter().enumerate() { + region.assign_fixed(|| "table col", self.config.sl, index, || Ok(value))?; } Ok(()) }, @@ -205,7 +191,7 @@ fn main() { fn without_witnesses(&self) -> Self { Self { a: None, - lookup_tables: self.lookup_tables.clone(), + lookup_table: self.lookup_table.clone(), } } @@ -226,34 +212,25 @@ fn main() { let sb = meta.fixed_column(); let sc = meta.fixed_column(); let sl = meta.fixed_column(); - let sl2 = meta.fixed_column(); /* - * A B ... sl sl2 + * A B ... sl * [ - * instance 0 ... 0 0 - * a a ... 0 0 - * a a^2 ... 0 0 - * a a ... 0 0 - * a a^2 ... 0 0 - * ... ... ... ... ... - * ... ... ... instance 0 - * ... ... ... a a - * ... ... ... a a^2 - * ... ... ... 0 0 + * instance 0 ... 0 + * a a ... 0 + * a a^2 ... 0 + * a a ... 0 + * a a^2 ... 0 + * ... ... ... ... + * ... ... ... instance + * ... ... ... a + * ... ... ... a + * ... ... ... 0 * ] */ meta.lookup(|meta| { let a_ = meta.query_any(a.into(), Rotation::cur()); - let sl_ = meta.query_any(sl.into(), Rotation::cur()); - vec![(a_, sl_)] - }); - meta.lookup(|meta| { - let a_ = meta.query_any(a.into(), Rotation::cur()); - let b_ = meta.query_any(b.into(), Rotation::cur()); - let sl_ = meta.query_any(sl.into(), Rotation::cur()); - let sl2_ = meta.query_any(sl2.into(), Rotation::cur()); - vec![(a_, sl_), (b_, sl2_)] + vec![(a_, sl)] }); meta.create_gate("Combined add-mult", |meta| { @@ -289,7 +266,6 @@ fn main() { sc, sm, sl, - sl2, } } @@ -327,21 +303,19 @@ fn main() { )?; } - cs.lookup_table(&mut layouter, &self.lookup_tables)?; + cs.lookup_table(&mut layouter, &self.lookup_table)?; Ok(()) } } let a = Fp::rand(); - let a_squared = a * a; let instance = Fp::one() + Fp::one(); let lookup_table = vec![instance, a, a, Fp::zero()]; - let lookup_table_2 = vec![Fp::zero(), a, a_squared, Fp::zero()]; let circuit: MyCircuit = MyCircuit { a: None, - lookup_tables: vec![lookup_table, lookup_table_2], + lookup_table: lookup_table, }; let root = BitMapBackend::new("example-circuit-layout.png", (1024, 768)).into_drawing_area(); From e23d148b72ebc8d1a37f10b72a80a8adf2a0711e Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Thu, 22 Jul 2021 12:07:17 -0600 Subject: [PATCH 11/19] minor clippy fixes --- examples/circuit-layout.rs | 2 +- src/circuit/floor_planner/v1/strategy.rs | 6 +++--- tests/plonk_api.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/examples/circuit-layout.rs b/examples/circuit-layout.rs index 5593b44b..d28b2676 100644 --- a/examples/circuit-layout.rs +++ b/examples/circuit-layout.rs @@ -315,7 +315,7 @@ fn main() { let circuit: MyCircuit = MyCircuit { a: None, - lookup_table: lookup_table, + lookup_table, }; let root = BitMapBackend::new("example-circuit-layout.png", (1024, 768)).into_drawing_area(); diff --git a/src/circuit/floor_planner/v1/strategy.rs b/src/circuit/floor_planner/v1/strategy.rs index 8b43e205..478fe8ee 100644 --- a/src/circuit/floor_planner/v1/strategy.rs +++ b/src/circuit/floor_planner/v1/strategy.rs @@ -233,7 +233,7 @@ fn test_slot_in() { region_index: 0.into(), columns: vec![Column::new(0, Any::Advice), Column::new(1, Any::Advice)] .into_iter() - .map(|a| Column::::from(a).into()) + .map(|a| a.into()) .collect(), row_count: 15, }, @@ -241,7 +241,7 @@ fn test_slot_in() { region_index: 1.into(), columns: vec![Column::new(2, Any::Advice)] .into_iter() - .map(|a| Column::::from(a).into()) + .map(|a| a.into()) .collect(), row_count: 10, }, @@ -249,7 +249,7 @@ fn test_slot_in() { region_index: 2.into(), columns: vec![Column::new(2, Any::Advice), Column::new(0, Any::Advice)] .into_iter() - .map(|a| Column::::from(a).into()) + .map(|a| a.into()) .collect(), row_count: 10, }, diff --git a/tests/plonk_api.rs b/tests/plonk_api.rs index dfa65cd9..d738303a 100644 --- a/tests/plonk_api.rs +++ b/tests/plonk_api.rs @@ -397,7 +397,7 @@ fn plonk_api() { let circuit: MyCircuit = MyCircuit { a: Some(a), - lookup_table: lookup_table, + lookup_table, }; // Initialize the proving key From 28439414f4b0a6bb060ad583957f73b6c44879d2 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Thu, 22 Jul 2021 16:36:28 -0600 Subject: [PATCH 12/19] (attempt 1 at implementing daira's algorithm) --- src/plonk/circuit.rs | 244 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 217 insertions(+), 27 deletions(-) diff --git a/src/plonk/circuit.rs b/src/plonk/circuit.rs index 190e7d3f..f58c0e7d 100644 --- a/src/plonk/circuit.rs +++ b/src/plonk/circuit.rs @@ -709,6 +709,32 @@ impl Expression { &|a, _| a, ) } + + /// Extracts a simple selector from this gate, if present + fn extract_simple_selector(&self) -> Option { + let op = |a, b| match (a, b) { + (Some(a), None) | (None, Some(a)) => Some(a), + (Some(_), Some(_)) => panic!("two simple selectors cannot be in the same expression"), + _ => None, + }; + + self.evaluate( + &|_| None, + &|selector| { + if selector.is_simple() { + Some(selector) + } else { + None + } + }, + &|_, _, _| None, + &|_, _, _| None, + &|_, _, _| None, + &op, + &op, + &|a, _| a, + ) + } } impl Neg for Expression { @@ -1138,34 +1164,202 @@ impl ConstraintSystem { /// /// Do not call this twice. Yes, this should be a builder pattern instead. pub(crate) fn compress_selectors(mut self, selectors: Vec>) -> (Self, Vec>) { - // Note: This is a stub for an actual optimization step. For now, just - // make new columns like we were doing already. + if selectors.is_empty() { + return (self, vec![]); + } + // The length of all provided selectors must be the same. + let n = selectors[0].len(); + assert!(selectors.iter().all(|a| a.len() == n)); + // and the number of provided selectors must be the same. assert_eq!(selectors.len(), self.num_selectors); - // Create self.num_selectors new fixed columns - let mut queries = vec![]; - for _ in 0..self.num_selectors { - let column = self.fixed_column(); - // Selectors are only queried here. - let query = self.query_fixed_index(column, Rotation::cur()); - queries.push(query); - self.selector_map.push(column); + // Compute the maximal degree of every simple selector. We only consider + // the expressions in gates, as lookup arguments cannot support simple + // selectors. + let mut degrees = vec![0; selectors.len()]; + for expr in self.gates.iter().flat_map(|gate| gate.polys.iter()) { + let degree = expr.degree(); + if let Some(selector) = expr.extract_simple_selector() { + degrees[selector.0] = max(degrees[selector.0], degree); + } } - let polys = selectors + // Any elements of `degrees` which still has zero must either be a + // simple selector or appear in no columns, so we don't need to compute + // anything for those. They will appear on their own. + let complex_selectors = degrees.iter().map(|&i| i == 0).collect::>(); + + // Compute the exclusion matrix that has (j, k) = true if selector j and + // selector k conflict -- that is, they are both enabled on the same + // row. This matrix is symmetric so we only need to store the lower + // diagonal. + let mut exclusion_matrix = (0..self.num_selectors) + .map(|i| vec![false; i]) + .collect::>(); + + // Iterate over each of the selector columns + for (i, rows) in selectors.iter().enumerate() { + // Ignore exclusions with complex selectors + if complex_selectors[i] { + continue; + } + + // Iterate over the rows, filtering for rows that are set + for set_index in rows.iter().enumerate().filter(|a| *a.1).map(|a| a.0) { + // Loop, looking only at the selectors previous to this one + for (j, other_selector) in selectors.iter().enumerate().take(i) { + // Ignore exclusions with complex selectors + if complex_selectors[j] { + continue; + } + + // Look at what selectors are active at the same row + if other_selector[set_index] { + // Mark them as incompatible + exclusion_matrix[i][j] = true; + } + } + } + } + + // Keep track of the combinations; the ith element of this vector + // represents the ith combination and contains a vector of (simple) + // selectors that are included. + let mut combinations = vec![]; + + // We will not increase the degree of the constraint system, so we limit + // ourselves to the largest existing degree constraint. + let max_degree = self.degree(); + + // Simple selectors that we've added to combinations already. + let mut seen = vec![false; self.num_selectors]; + + let mut selector_map = vec![0; self.num_selectors]; + + for i in 0..self.num_selectors { + if complex_selectors[i] { + selector_map[i] = combinations.len(); + // Complex selectors are on their own. + combinations.push(vec![i]); + + // NB: We don't update `seen` here as it's a complex selector + // that will be ignored anyway. + } else { + // This is a simple selector. + if seen[i] { + // This simple selector was added to a previous combination. + continue; + } + selector_map[i] = combinations.len(); + seen[i] = true; // We're definitely adding this to a combination. + let mut combination = vec![i]; + assert!(degrees[i] <= max_degree); // Otherwise, self.degree() is wrong. + + // Keep track of how large the substitution is. + let mut usage = 1; + + // Keep track of how large of a gate any of the members of this + // combination can support. + let mut room = max_degree - degrees[i]; + + // Try to find other selectors that can join this one. + for j in (i + 1)..self.num_selectors { + // Is there room for the new selector? + if room == 0 { + // Short circuit here, we've hit our limit. + break; + } + + // Skip complex selectors. + if complex_selectors[j] { + continue; + } + + // Skip selectors that have been added to previous combinations + if seen[j] { + continue; + } + + // Are the two selectors excluded from co-existing? + // j > i and so (j, i) is always on the lower diagonal + if exclusion_matrix[j][i] { + continue; + } + + // Can the new selector join the combination? + if degrees[j] + usage <= max_degree { + // Yes, it can. Let's do it. + combination.push(j); + // The remaining room available is the lesser of the + // current remaining room available for previous + // selector(s) after adjusting for adding this new + // selector, and the room available in the new selector + // after adjusting for adding the previous selectors. + room = std::cmp::min(room - 1, max_degree - (degrees[j] + usage)); + usage += 1; + selector_map[j] = combinations.len(); + seen[j] = true; + } + } + + combinations.push(combination); + } + } + + let mut selector_replacements = vec![None; self.num_selectors]; + let mut polys = vec![vec![F::zero(); n]; combinations.len()]; + let mut new_columns = vec![]; + for (combination, poly) in combinations.iter().zip(polys.iter_mut()) { + // Create a new fixed column for the logical selector + let column = self.fixed_column(); + new_columns.push(Some(column)); + let query = Expression::Fixed { + query_index: self.query_fixed_index(column, Rotation::cur()), + column_index: column.index, + rotation: Rotation::cur(), + }; + + // Every virtual selector gets a different integer (starting at 1) + // that it assigns when it wants to be active. + let mut assignment = F::one(); + for &selector in combination { + for (s, poly) in selectors[selector].iter().zip(poly.iter_mut()) { + if *s { + *poly = assignment; + } + } + // Construct a polynomial that has a root at 0 and at integers + // not including what this selector has been assigned to. This + // is a generalization of a boolean selector when the selector + // is multiplying a value that is constrained to equal zero. + let mut expr = query.clone(); + let mut root = F::one(); + for _ in 0..combination.len() { + if root != assignment { + expr = expr * (query.clone() - Expression::Constant(root)); + } + root += F::one(); + } + + assert!(selector_replacements[selector].is_none()); + selector_replacements[selector] = Some(expr); + + assignment += F::one(); + } + } + + // All selectors have a combination now. + for selector in selector_map { + self.selector_map.push(new_columns[selector].unwrap()); + } + let selector_replacements = selector_replacements .into_iter() - .map(|bools| { - bools - .into_iter() - .map(|coeff| if coeff { F::one() } else { F::zero() }) - .collect() - }) - .collect(); + .map(|a| a.unwrap()) + .collect::>(); fn replace_selectors( expr: &mut Expression, - selector_map: &[Column], - selector_queries: &[usize], + selector_map: &[Expression], must_be_nonsimple: bool, ) { *expr = expr.evaluate( @@ -1178,11 +1372,7 @@ impl ConstraintSystem { assert!(!selector.is_simple()); } - Expression::Fixed { - query_index: selector_queries[selector.0], - column_index: selector_map[selector.0].index(), - rotation: Rotation::cur(), - } + selector_map[selector.0].clone() }, &|query_index, column_index, rotation| Expression::Fixed { query_index, @@ -1207,7 +1397,7 @@ impl ConstraintSystem { // Substitute selectors for the real fixed columns in all gates for expr in self.gates.iter_mut().flat_map(|gate| gate.polys.iter_mut()) { - replace_selectors(expr, &self.selector_map, &queries, false); + replace_selectors(expr, &selector_replacements, false); } // Substitute non-simple selectors for the real fixed columns in all @@ -1218,7 +1408,7 @@ impl ConstraintSystem { .iter_mut() .chain(lookup.table_expressions.iter_mut()) }) { - replace_selectors(expr, &self.selector_map, &queries, true); + replace_selectors(expr, &selector_replacements, true); } (self, polys) From 754dd62c92e39a1c98113d8381b3af7a973dceb0 Mon Sep 17 00:00:00 2001 From: ebfull Date: Sat, 24 Jul 2021 10:27:50 -0600 Subject: [PATCH 13/19] Apply suggestions from code review Co-authored-by: str4d Co-authored-by: Daira Hopwood --- examples/circuit-layout.rs | 2 +- src/circuit/layouter.rs | 10 +++++----- src/plonk/circuit.rs | 24 +++++++++++++----------- src/plonk/lookup/prover.rs | 2 +- src/plonk/lookup/verifier.rs | 2 +- 5 files changed, 21 insertions(+), 19 deletions(-) diff --git a/examples/circuit-layout.rs b/examples/circuit-layout.rs index d28b2676..152a97e7 100644 --- a/examples/circuit-layout.rs +++ b/examples/circuit-layout.rs @@ -325,6 +325,6 @@ fn main() { .unwrap(); CircuitLayout::default() - .render(1 << 5, &circuit, &root) + .render(5, &circuit, &root) .unwrap(); } diff --git a/src/circuit/layouter.rs b/src/circuit/layouter.rs index f020688a..4838b02b 100644 --- a/src/circuit/layouter.rs +++ b/src/circuit/layouter.rs @@ -114,11 +114,11 @@ pub struct RegionShape { pub(super) row_count: usize, } -/// The virtual column involved in a region. This includes normal "logical" -/// columns as well as selectors that are not definite columns at this stage. +/// The virtual column involved in a region. This includes concrete columns, +/// as well as selectors that are not concrete columns at this stage. #[derive(Eq, PartialEq, Copy, Clone, Debug, Hash)] pub enum RegionColumn { - /// Logical column + /// Concrete column Column(Column), /// Virtual column representing a (boolean) selector Selector(Selector), @@ -141,8 +141,8 @@ impl Ord for RegionColumn { match (self, other) { (Self::Column(ref a), Self::Column(ref b)) => a.cmp(b), (Self::Selector(ref a), Self::Selector(ref b)) => a.0.cmp(&b.0), - (Self::Column(_), Self::Selector(_)) => cmp::Ordering::Greater, - (Self::Selector(_), Self::Column(_)) => cmp::Ordering::Less, + (Self::Column(_), Self::Selector(_)) => cmp::Ordering::Less, + (Self::Selector(_), Self::Column(_)) => cmp::Ordering::Greater, } } } diff --git a/src/plonk/circuit.rs b/src/plonk/circuit.rs index f58c0e7d..cb4fac62 100644 --- a/src/plonk/circuit.rs +++ b/src/plonk/circuit.rs @@ -696,7 +696,7 @@ impl Expression { self.clone() * self } - /// Returns whether or not this expression contains a `Selector`. + /// Returns whether or not this expression contains a simple `Selector`. fn contains_simple_selector(&self) -> bool { self.evaluate( &|_| false, @@ -748,7 +748,7 @@ impl Add for Expression { type Output = Expression; fn add(self, rhs: Expression) -> Expression { if self.contains_simple_selector() || rhs.contains_simple_selector() { - panic!("attempted to add to a simple selector"); + panic!("attempted to use a simple selector in an addition"); } Expression::Sum(Box::new(self), Box::new(rhs)) } @@ -758,7 +758,7 @@ impl Sub for Expression { type Output = Expression; fn sub(self, rhs: Expression) -> Expression { if self.contains_simple_selector() || rhs.contains_simple_selector() { - panic!("attempted to add to a simple selector"); + panic!("attempted to use a simple selector in a subtraction"); } Expression::Sum(Box::new(self), Box::new(-rhs)) } @@ -768,7 +768,7 @@ impl Mul for Expression { type Output = Expression; fn mul(self, rhs: Expression) -> Expression { if self.contains_simple_selector() && rhs.contains_simple_selector() { - panic!("attempted to multiply two simple selectors"); + panic!("attempted to multiply two expressions containing simple selectors"); } Expression::Product(Box::new(self), Box::new(rhs)) } @@ -1170,7 +1170,8 @@ impl ConstraintSystem { // The length of all provided selectors must be the same. let n = selectors[0].len(); assert!(selectors.iter().all(|a| a.len() == n)); - // and the number of provided selectors must be the same. + // and the number of provided selectors must be the number we counted + // for this ConstraintSystem. assert_eq!(selectors.len(), self.num_selectors); // Compute the maximal degree of every simple selector. We only consider @@ -1184,15 +1185,15 @@ impl ConstraintSystem { } } - // Any elements of `degrees` which still has zero must either be a - // simple selector or appear in no columns, so we don't need to compute + // Any element of `degrees` which still has zero must either be a + // complex selector or appear in no columns, so we don't need to compute // anything for those. They will appear on their own. let complex_selectors = degrees.iter().map(|&i| i == 0).collect::>(); // Compute the exclusion matrix that has (j, k) = true if selector j and // selector k conflict -- that is, they are both enabled on the same - // row. This matrix is symmetric so we only need to store the lower - // diagonal. + // row. This matrix is symmetric and the diagonal entries are false, so + // we only need to store the lower triangular entries. let mut exclusion_matrix = (0..self.num_selectors) .map(|i| vec![false; i]) .collect::>(); @@ -1336,7 +1337,7 @@ impl ConstraintSystem { let mut root = F::one(); for _ in 0..combination.len() { if root != assignment { - expr = expr * (query.clone() - Expression::Constant(root)); + expr = expr * (Expression::Constant(root) - query.clone()); } root += F::one(); } @@ -1366,7 +1367,8 @@ impl ConstraintSystem { &|constant| Expression::Constant(constant), &|selector| { if must_be_nonsimple { - // Complex selectors are prohibited from appearing in + // Simple selectors are prohibited from appearing in +``` // expressions in the lookup argument by // `ConstraintSystem`. assert!(!selector.is_simple()); diff --git a/src/plonk/lookup/prover.rs b/src/plonk/lookup/prover.rs index 7c9dd40a..3377c43f 100644 --- a/src/plonk/lookup/prover.rs +++ b/src/plonk/lookup/prover.rs @@ -135,7 +135,7 @@ impl Argument { .map(|expression| { expression.evaluate( &|scalar| pk.vk.domain.constant_extended(scalar), - &|_| panic!("virtual selectors are optimized away during keygen"), + &|_| panic!("virtual selectors are removed during optimization"), &|_, column_index, rotation| { pk.vk .domain diff --git a/src/plonk/lookup/verifier.rs b/src/plonk/lookup/verifier.rs index e33e5066..70c10b7f 100644 --- a/src/plonk/lookup/verifier.rs +++ b/src/plonk/lookup/verifier.rs @@ -134,7 +134,7 @@ impl Evaluated { .map(|expression| { expression.evaluate( &|scalar| scalar, - &|_| panic!("virtual selectors are optimized away during keygen"), + &|_| panic!("virtual selectors are removed during optimization"), &|index, _, _| fixed_evals[index], &|index, _, _| advice_evals[index], &|index, _, _| instance_evals[index], From 634d14a01680c844f3b94949a331a3ccea84d981 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Sat, 24 Jul 2021 10:43:50 -0600 Subject: [PATCH 14/19] Remove spurious code from review suggestion. --- src/plonk/circuit.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/plonk/circuit.rs b/src/plonk/circuit.rs index cb4fac62..dae63719 100644 --- a/src/plonk/circuit.rs +++ b/src/plonk/circuit.rs @@ -1368,7 +1368,6 @@ impl ConstraintSystem { &|selector| { if must_be_nonsimple { // Simple selectors are prohibited from appearing in -``` // expressions in the lookup argument by // `ConstraintSystem`. assert!(!selector.is_simple()); From e02fc06f52e400e1da5e0ef294a4d691e14f74a9 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Sat, 24 Jul 2021 15:42:30 -0600 Subject: [PATCH 15/19] cargo fmt --- examples/circuit-layout.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/examples/circuit-layout.rs b/examples/circuit-layout.rs index 152a97e7..10d40b6b 100644 --- a/examples/circuit-layout.rs +++ b/examples/circuit-layout.rs @@ -324,7 +324,5 @@ fn main() { .titled("Example Circuit Layout", ("sans-serif", 60)) .unwrap(); - CircuitLayout::default() - .render(5, &circuit, &root) - .unwrap(); + CircuitLayout::default().render(5, &circuit, &root).unwrap(); } From 2a253b89b0020685215f0cc8c6099bfe3508a671 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Sat, 24 Jul 2021 15:43:15 -0600 Subject: [PATCH 16/19] Refactor the selector optimization pass so it can be independently proptested. --- Cargo.toml | 1 + .../plonk/circuit/compress_selectors.txt | 9 + src/plonk/circuit.rs | 214 +++-------- src/plonk/circuit/compress_selectors.rs | 341 ++++++++++++++++++ 4 files changed, 393 insertions(+), 172 deletions(-) create mode 100644 proptest-regressions/plonk/circuit/compress_selectors.txt create mode 100644 src/plonk/circuit/compress_selectors.rs diff --git a/Cargo.toml b/Cargo.toml index b9721146..738b7c93 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,6 +51,7 @@ tabbycat = { version = "0.1", features = ["attributes"], optional = true } [dev-dependencies] criterion = "0.3" gumdrop = "0.8" +proptest = "1" [features] dev-graph = ["plotters", "tabbycat"] diff --git a/proptest-regressions/plonk/circuit/compress_selectors.txt b/proptest-regressions/plonk/circuit/compress_selectors.txt new file mode 100644 index 00000000..22a5b484 --- /dev/null +++ b/proptest-regressions/plonk/circuit/compress_selectors.txt @@ -0,0 +1,9 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc 782948e336b9fcaaf993d40cd290eff20399d34766a93793fc3a4516274c1ea7 # shrinks to (selectors, max_degree) = ([SelectorDescription { selector: 0, activations: [false], max_degree: 0 }, SelectorDescription { selector: 1, activations: [false], max_degree: 0 }], 1) +cc 656e5446792c4f5fe22fd10bcd2dbadc70e84ac1ddb1a7ec8f622f64a15ff260 # shrinks to (selectors, max_degree) = ([SelectorDescription { selector: 0, activations: [false], max_degree: 1 }, SelectorDescription { selector: 1, activations: [false], max_degree: 1 }, SelectorDescription { selector: 2, activations: [false], max_degree: 1 }], 2) +cc b7b81ca8745931e4dd8b4f896f7bde78f85f4d88857c5fdf9dc4bbf0f172db5e # shrinks to (selectors, max_degree) = ([SelectorDescription { selector: 0, activations: [false], max_degree: 1 }, SelectorDescription { selector: 1, activations: [false], max_degree: 1 }, SelectorDescription { selector: 2, activations: [false], max_degree: 1 }], 2) diff --git a/src/plonk/circuit.rs b/src/plonk/circuit.rs index dae63719..f8d66bbb 100644 --- a/src/plonk/circuit.rs +++ b/src/plonk/circuit.rs @@ -10,6 +10,8 @@ use super::{lookup, permutation, Error}; use crate::circuit::Layouter; use crate::{circuit::Region, poly::Rotation}; +mod compress_selectors; + /// A column type pub trait ColumnType: 'static + Sized + Copy + std::fmt::Debug + PartialEq + Eq + Into @@ -1164,19 +1166,14 @@ impl ConstraintSystem { /// /// Do not call this twice. Yes, this should be a builder pattern instead. pub(crate) fn compress_selectors(mut self, selectors: Vec>) -> (Self, Vec>) { - if selectors.is_empty() { - return (self, vec![]); - } - // The length of all provided selectors must be the same. - let n = selectors[0].len(); - assert!(selectors.iter().all(|a| a.len() == n)); - // and the number of provided selectors must be the number we counted - // for this ConstraintSystem. + // The number of provided selector assignments must be the number we + // counted for this constraint system. assert_eq!(selectors.len(), self.num_selectors); - // Compute the maximal degree of every simple selector. We only consider - // the expressions in gates, as lookup arguments cannot support simple - // selectors. + // Compute the maximal degree of every selector. We only consider the + // expressions in gates, as lookup arguments cannot support simple + // selectors. Selectors that are complex or do not appear in any gates + // will have degree zero. let mut degrees = vec![0; selectors.len()]; for expr in self.gates.iter().flat_map(|gate| gate.polys.iter()) { let degree = expr.degree(); @@ -1185,174 +1182,47 @@ impl ConstraintSystem { } } - // Any element of `degrees` which still has zero must either be a - // complex selector or appear in no columns, so we don't need to compute - // anything for those. They will appear on their own. - let complex_selectors = degrees.iter().map(|&i| i == 0).collect::>(); - - // Compute the exclusion matrix that has (j, k) = true if selector j and - // selector k conflict -- that is, they are both enabled on the same - // row. This matrix is symmetric and the diagonal entries are false, so - // we only need to store the lower triangular entries. - let mut exclusion_matrix = (0..self.num_selectors) - .map(|i| vec![false; i]) - .collect::>(); - - // Iterate over each of the selector columns - for (i, rows) in selectors.iter().enumerate() { - // Ignore exclusions with complex selectors - if complex_selectors[i] { - continue; - } - - // Iterate over the rows, filtering for rows that are set - for set_index in rows.iter().enumerate().filter(|a| *a.1).map(|a| a.0) { - // Loop, looking only at the selectors previous to this one - for (j, other_selector) in selectors.iter().enumerate().take(i) { - // Ignore exclusions with complex selectors - if complex_selectors[j] { - continue; - } - - // Look at what selectors are active at the same row - if other_selector[set_index] { - // Mark them as incompatible - exclusion_matrix[i][j] = true; - } - } - } - } - - // Keep track of the combinations; the ith element of this vector - // represents the ith combination and contains a vector of (simple) - // selectors that are included. - let mut combinations = vec![]; - // We will not increase the degree of the constraint system, so we limit // ourselves to the largest existing degree constraint. let max_degree = self.degree(); - // Simple selectors that we've added to combinations already. - let mut seen = vec![false; self.num_selectors]; - - let mut selector_map = vec![0; self.num_selectors]; - - for i in 0..self.num_selectors { - if complex_selectors[i] { - selector_map[i] = combinations.len(); - // Complex selectors are on their own. - combinations.push(vec![i]); - - // NB: We don't update `seen` here as it's a complex selector - // that will be ignored anyway. - } else { - // This is a simple selector. - if seen[i] { - // This simple selector was added to a previous combination. - continue; - } - selector_map[i] = combinations.len(); - seen[i] = true; // We're definitely adding this to a combination. - let mut combination = vec![i]; - assert!(degrees[i] <= max_degree); // Otherwise, self.degree() is wrong. - - // Keep track of how large the substitution is. - let mut usage = 1; - - // Keep track of how large of a gate any of the members of this - // combination can support. - let mut room = max_degree - degrees[i]; - - // Try to find other selectors that can join this one. - for j in (i + 1)..self.num_selectors { - // Is there room for the new selector? - if room == 0 { - // Short circuit here, we've hit our limit. - break; - } - - // Skip complex selectors. - if complex_selectors[j] { - continue; - } - - // Skip selectors that have been added to previous combinations - if seen[j] { - continue; - } - - // Are the two selectors excluded from co-existing? - // j > i and so (j, i) is always on the lower diagonal - if exclusion_matrix[j][i] { - continue; - } - - // Can the new selector join the combination? - if degrees[j] + usage <= max_degree { - // Yes, it can. Let's do it. - combination.push(j); - // The remaining room available is the lesser of the - // current remaining room available for previous - // selector(s) after adjusting for adding this new - // selector, and the room available in the new selector - // after adjusting for adding the previous selectors. - room = std::cmp::min(room - 1, max_degree - (degrees[j] + usage)); - usage += 1; - selector_map[j] = combinations.len(); - seen[j] = true; - } - } - - combinations.push(combination); - } - } - - let mut selector_replacements = vec![None; self.num_selectors]; - let mut polys = vec![vec![F::zero(); n]; combinations.len()]; let mut new_columns = vec![]; - for (combination, poly) in combinations.iter().zip(polys.iter_mut()) { - // Create a new fixed column for the logical selector - let column = self.fixed_column(); - new_columns.push(Some(column)); - let query = Expression::Fixed { - query_index: self.query_fixed_index(column, Rotation::cur()), - column_index: column.index, - rotation: Rotation::cur(), - }; - - // Every virtual selector gets a different integer (starting at 1) - // that it assigns when it wants to be active. - let mut assignment = F::one(); - for &selector in combination { - for (s, poly) in selectors[selector].iter().zip(poly.iter_mut()) { - if *s { - *poly = assignment; - } - } - // Construct a polynomial that has a root at 0 and at integers - // not including what this selector has been assigned to. This - // is a generalization of a boolean selector when the selector - // is multiplying a value that is constrained to equal zero. - let mut expr = query.clone(); - let mut root = F::one(); - for _ in 0..combination.len() { - if root != assignment { - expr = expr * (Expression::Constant(root) - query.clone()); - } - root += F::one(); + let (polys, selector_assignment) = compress_selectors::process( + selectors + .into_iter() + .zip(degrees.into_iter()) + .enumerate() + .map( + |(i, (activations, max_degree))| compress_selectors::SelectorDescription { + selector: i, + activations, + max_degree, + }, + ) + .collect(), + max_degree, + || { + let column = self.fixed_column(); + new_columns.push(column); + Expression::Fixed { + query_index: self.query_fixed_index(column, Rotation::cur()), + column_index: column.index, + rotation: Rotation::cur(), } + }, + ); - assert!(selector_replacements[selector].is_none()); - selector_replacements[selector] = Some(expr); - - assignment += F::one(); - } + let mut selector_map = vec![None; selector_assignment.len()]; + let mut selector_replacements = vec![None; selector_assignment.len()]; + for assignment in selector_assignment { + selector_replacements[assignment.selector] = Some(assignment.expression); + selector_map[assignment.selector] = Some(new_columns[assignment.combination_index]); } - // All selectors have a combination now. - for selector in selector_map { - self.selector_map.push(new_columns[selector].unwrap()); - } + self.selector_map = selector_map + .into_iter() + .map(|a| a.unwrap()) + .collect::>(); let selector_replacements = selector_replacements .into_iter() .map(|a| a.unwrap()) @@ -1360,7 +1230,7 @@ impl ConstraintSystem { fn replace_selectors( expr: &mut Expression, - selector_map: &[Expression], + selector_replacements: &[Expression], must_be_nonsimple: bool, ) { *expr = expr.evaluate( @@ -1373,7 +1243,7 @@ impl ConstraintSystem { assert!(!selector.is_simple()); } - selector_map[selector.0].clone() + selector_replacements[selector.0].clone() }, &|query_index, column_index, rotation| Expression::Fixed { query_index, diff --git a/src/plonk/circuit/compress_selectors.rs b/src/plonk/circuit/compress_selectors.rs new file mode 100644 index 00000000..83fb6493 --- /dev/null +++ b/src/plonk/circuit/compress_selectors.rs @@ -0,0 +1,341 @@ +use super::Expression; +use ff::Field; + +/// This describes a selector and where it is activated. +#[derive(Debug, Clone)] +pub struct SelectorDescription { + /// The selector that this description references, by index. + pub selector: usize, + + /// The vector of booleans defining which rows are active for this selector. + pub activations: Vec, + + /// The maximum degree of a gate involving this selector, including the + /// virtual selector itself. This means this will be at least 1 for any + /// expression containing a simple selector, even if that selector is not + /// multiplied by anything. + pub max_degree: usize, +} + +/// This describes the assigned combination of a particular selector as well as +/// the expression it should be substituted with. +#[derive(Debug, Clone)] +pub struct SelectorAssignment { + /// The selector that this structure references, by index. + pub selector: usize, + + /// The combination this selector was assigned to + pub combination_index: usize, + + /// The expression we wish to substitute with + pub expression: Expression, +} + +/// This function takes a vector that defines each selector as well as a closure +/// used to allocate new fixed columns, and returns the assignment of each +/// combination as well as details about each selector assignment. +/// +/// This function takes +/// * `selectors`, a vector of `SelectorDescription`s that describe each +/// selector +/// * `max_degree`, the maximum allowed degree of any gate +/// * `allocate_fixed_columns`, a closure that constructs a new fixed column and +/// queries it at Rotation::cur(), returning the expression +/// +/// and returns `Vec>` containing the assignment of each new fixed column +/// (which each correspond to a combination) as well as a vector of +/// `SelectorAssignment` that the caller can use to perform the necessary +/// substitutions to the constraint system. +/// +/// This function is completely deterministic. +pub fn process( + mut selectors: Vec, + max_degree: usize, + mut allocate_fixed_column: E, +) -> (Vec>, Vec>) +where + E: FnMut() -> Expression, +{ + if selectors.is_empty() { + // There is nothing to optimize. + return (vec![], vec![]); + } + + // The length of all provided selectors must be the same. + let n = selectors[0].activations.len(); + assert!(selectors.iter().all(|a| a.activations.len() == n)); + + let mut combination_assignments = vec![]; + let mut selector_assignments = vec![]; + + // All provided selectors of degree 0 are assumed to be either concrete + // selectors or do not appear in a gate. Let's address these first. + selectors = selectors + .into_iter() + .filter(|selector| { + if selector.max_degree == 0 { + // This is a concrete selector. + let expression = allocate_fixed_column(); + + let combination_assignment = selector + .activations + .iter() + .map(|b| if *b { F::one() } else { F::zero() }) + .collect::>(); + let combination_index = combination_assignments.len(); + combination_assignments.push(combination_assignment); + selector_assignments.push(SelectorAssignment { + selector: selector.selector, + combination_index, + expression, + }); + + false + } else { + true + } + }) + .collect(); + + // All of the remaining `selectors` are simple. Let's try to combine them. + // First, we compute the exclusion matrix that has (j, k) = true if selector + // j and selector k conflict -- that is, they are both enabled on the same + // row. This matrix is symmetric and the diagonal entries are false, so we + // only need to store the lower triangular entries. + let mut exclusion_matrix = (0..selectors.len()) + .map(|i| vec![false; i]) + .collect::>(); + + for (i, rows) in selectors + .iter() + .map(|selector| &selector.activations) + .enumerate() + { + // Iterate over the rows, filtering for rows that are set + for set_index in rows.iter().enumerate().filter(|a| *a.1).map(|a| a.0) { + // Loop over the selectors previous to this one + for (j, other_selector) in selectors.iter().enumerate().take(i) { + // Look at what selectors are active at the same row + if other_selector.activations[set_index] { + // Mark them as incompatible + exclusion_matrix[i][j] = true; + } + } + } + } + + // Simple selectors that we've added to combinations already. + let mut added = vec![false; selectors.len()]; + + for (i, selector) in selectors.iter().enumerate() { + if added[i] { + continue; + } + added[i] = true; + assert!(selector.max_degree <= max_degree); + // This is used to keep track of the largest degree gate involved in the + // combination so far. We subtract by one to omit the virtual selector + // which will be substituted by the caller with the expression we give + // them. + let mut d = selector.max_degree - 1; + let mut combination = vec![selector]; + let mut combination_added = vec![i]; + + // Try to find other selectors that can join this one. + 'try_selectors: for (j, selector) in selectors.iter().enumerate().skip(i + 1) { + if d == max_degree { + // Short circuit; nothing can be added to this + // combination. + break 'try_selectors; + } + + // Skip selectors that have been added to previous combinations + if added[j] { + continue 'try_selectors; + } + + // Is this selector excluded from co-existing in the same + // combination with any of the other selectors so far? + for &i in combination_added.iter() { + if exclusion_matrix[j][i] { + continue 'try_selectors; + } + } + + // Can the new selector join the combination? Reminder: we subtract + // selector.max_degree - 1 to omit the influence of the virtual + // selector on the degree, as it will be substituted. + let new_d = std::cmp::max(d, selector.max_degree - 1); + if new_d + combination.len() + 1 > max_degree { + // Guess not. + continue 'try_selectors; + } + + d = new_d; + combination.push(selector); + combination_added.push(j); + added[j] = true; + } + + // Now, compute the selector and combination assignments. + let mut combination_assignment = vec![F::zero(); n]; + let combination_len = combination.len(); + let combination_index = combination_assignments.len(); + let query = allocate_fixed_column(); + + let mut assigned_root = F::one(); + selector_assignments.extend(combination.into_iter().map(|selector| { + // Compute the expression for substitution + let mut expression = query.clone(); + let mut root = F::one(); + for _ in 0..combination_len { + if root != assigned_root { + expression = expression * (Expression::Constant(root) - query.clone()); + } + root += F::one(); + } + + // Update the combination assignment + for (combination, selector) in combination_assignment + .iter_mut() + .zip(selector.activations.iter()) + { + // This will not overwrite another selector's activations because + // we have ensured that selectors are disjoint. + if *selector { + *combination = assigned_root; + } + } + + assigned_root += F::one(); + + SelectorAssignment { + selector: selector.selector, + combination_index, + expression, + } + })); + combination_assignments.push(combination_assignment); + } + + (combination_assignments, selector_assignments) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::poly::Rotation; + use pasta_curves::Fp; + use proptest::collection::{vec, SizeRange}; + use proptest::prelude::*; + + prop_compose! { + fn arb_selector(assignment_size: usize, max_degree: usize) + (degree in 0..max_degree, + assignment in vec(any::(), assignment_size)) + -> (usize, Vec) { + (degree, assignment) + } + } + + prop_compose! { + fn arb_selector_list(assignment_size: usize, max_degree: usize, num_selectors: impl Into) + (list in vec(arb_selector(assignment_size, max_degree), num_selectors)) + -> Vec + { + list.into_iter().enumerate().map(|(i, (max_degree, activations))| { + SelectorDescription { + selector: i, + activations, + max_degree, + } + }).collect() + } + } + + prop_compose! { + fn arb_instance(max_assignment_size: usize, + max_degree: usize, + max_selectors: usize) + (assignment_size in 1..max_assignment_size, + degree in 1..max_degree, + num_selectors in 1..max_selectors) + (list in arb_selector_list(assignment_size, degree, num_selectors), + degree in Just(degree)) + -> (Vec, usize) + { + (list, degree) + } + } + + proptest! { + #![proptest_config(ProptestConfig::with_cases(10000))] + #[test] + fn test_selector_combination((selectors, max_degree) in arb_instance(10, 10, 15)) { + let mut query = 0; + let (combination_assignments, selector_assignments) = + process::(selectors.clone(), max_degree, || { + let tmp = Expression::Fixed { + query_index: query, + column_index: query, + rotation: Rotation::cur(), + }; + query += 1; + tmp + }); + + { + let mut selectors_seen = vec![]; + assert_eq!(selectors.len(), selector_assignments.len()); + for selector in &selector_assignments { + // Every selector should be assigned to a combination + assert!(selector.combination_index < combination_assignments.len()); + assert!(!selectors_seen.contains(&selector.selector)); + selectors_seen.push(selector.selector); + } + } + + // Test that, for each selector, the provided expression + // 1. evaluates to zero on rows where the selector's activation is off + // 2. evaluates to nonzero on rows where the selector's activation is off + // 3. is of degree d such that d + (selector.max_degree - 1) <= max_degree + // OR selector.max_degree is zero + for selector in selector_assignments { + assert_eq!( + selectors[selector.selector].activations.len(), + combination_assignments[selector.combination_index].len() + ); + for (&activation, &assignment) in selectors[selector.selector].activations.iter().zip(combination_assignments[selector.combination_index].iter()) { + let eval = selector.expression.evaluate( + &|c| c, + &|_| panic!("should not occur in returned expressions"), + &|query_index, _, _| { + // Should be the correct combination in the expression + assert_eq!(selector.combination_index, query_index); + assignment + }, + &|_, _, _| panic!("should not occur in returned expressions"), + &|_, _, _| panic!("should not occur in returned expressions"), + &|a, b| a + b, + &|a, b| a * b, + &|a, f| a * f, + ); + + if activation { + assert!(!eval.is_zero()); + } else { + assert!(eval.is_zero()); + } + } + + let expr_degree = selector.expression.degree(); + assert!(expr_degree <= max_degree); + if selectors[selector.selector].max_degree > 0 { + assert!( + (selectors[selector.selector].max_degree - 1) + expr_degree <= max_degree + ); + } + } + } + } +} From 77805d9e8e3c7f413dfd74a5e0b24a44533c4892 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Sat, 24 Jul 2021 17:10:59 -0600 Subject: [PATCH 17/19] fix minor typo --- src/plonk/circuit/compress_selectors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plonk/circuit/compress_selectors.rs b/src/plonk/circuit/compress_selectors.rs index 83fb6493..00577bf7 100644 --- a/src/plonk/circuit/compress_selectors.rs +++ b/src/plonk/circuit/compress_selectors.rs @@ -297,7 +297,7 @@ mod tests { // Test that, for each selector, the provided expression // 1. evaluates to zero on rows where the selector's activation is off - // 2. evaluates to nonzero on rows where the selector's activation is off + // 2. evaluates to nonzero on rows where the selector's activation is on // 3. is of degree d such that d + (selector.max_degree - 1) <= max_degree // OR selector.max_degree is zero for selector in selector_assignments { From 88d34924eb5db3979e2f89615607b7c6e0847eb0 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Sat, 24 Jul 2021 17:16:14 -0600 Subject: [PATCH 18/19] Account for changes from #350. --- src/circuit/floor_planner/single_pass.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/circuit/floor_planner/single_pass.rs b/src/circuit/floor_planner/single_pass.rs index 4354a33e..1b388af0 100644 --- a/src/circuit/floor_planner/single_pass.rs +++ b/src/circuit/floor_planner/single_pass.rs @@ -11,7 +11,7 @@ use crate::{ Cell, Layouter, Region, RegionIndex, RegionStart, }, plonk::{ - Advice, Assigned, Assignment, Circuit, Column, Error, Fixed, FloorPlanner, Instance, + Advice, Any, Assigned, Assignment, Circuit, Column, Error, Fixed, FloorPlanner, Instance, Selector, }, }; @@ -119,7 +119,10 @@ impl<'a, F: Field, CS: Assignment + 'a> Layouter for SingleChipLayouter<'a } } else { let constants_column = self.constants[0]; - let next_constant_row = self.columns.entry(constants_column.into()).or_default(); + let next_constant_row = self + .columns + .entry(Column::::from(constants_column).into()) + .or_default(); for (constant, advice) in constants_to_assign { self.cs.assign_fixed( || format!("Constant({:?})", constant.evaluate()), From 101d7c79f3c8ac66c91a1a71345de56922eb8e81 Mon Sep 17 00:00:00 2001 From: ebfull Date: Sun, 25 Jul 2021 18:39:42 -0600 Subject: [PATCH 19/19] Apply suggestions from code review Co-authored-by: str4d --- src/dev/cost.rs | 6 +--- src/plonk/circuit.rs | 3 +- src/plonk/circuit/compress_selectors.rs | 40 ++++++++++++++++--------- 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/dev/cost.rs b/src/dev/cost.rs index ff899f19..18d5770b 100644 --- a/src/dev/cost.rs +++ b/src/dev/cost.rs @@ -10,7 +10,6 @@ use ff::{Field, PrimeField}; use group::prime::PrimeGroup; use crate::{ - arithmetic::FieldExt, plonk::{ Advice, Any, Assigned, Assignment, Circuit, Column, ConstraintSystem, Error, Fixed, FloorPlanner, Instance, Selector, @@ -125,10 +124,7 @@ impl> CircuitCost Self - where - G::Scalar: FieldExt, - { + pub fn measure(k: usize, circuit: &ConcreteCircuit) -> Self { // Collect the layout details. let mut cs = ConstraintSystem::default(); let config = ConcreteCircuit::configure(&mut cs); diff --git a/src/plonk/circuit.rs b/src/plonk/circuit.rs index f8d66bbb..fc458415 100644 --- a/src/plonk/circuit.rs +++ b/src/plonk/circuit.rs @@ -1176,9 +1176,8 @@ impl ConstraintSystem { // will have degree zero. let mut degrees = vec![0; selectors.len()]; for expr in self.gates.iter().flat_map(|gate| gate.polys.iter()) { - let degree = expr.degree(); if let Some(selector) = expr.extract_simple_selector() { - degrees[selector.0] = max(degrees[selector.0], degree); + degrees[selector.0] = max(degrees[selector.0], expr.degree()); } } diff --git a/src/plonk/circuit/compress_selectors.rs b/src/plonk/circuit/compress_selectors.rs index 00577bf7..9f390f4d 100644 --- a/src/plonk/circuit/compress_selectors.rs +++ b/src/plonk/circuit/compress_selectors.rs @@ -74,7 +74,8 @@ where .into_iter() .filter(|selector| { if selector.max_degree == 0 { - // This is a concrete selector. + // This is a complex selector, or a selector that does not appear in any + // gate constraint. let expression = allocate_fixed_column(); let combination_assignment = selector @@ -111,15 +112,16 @@ where .map(|selector| &selector.activations) .enumerate() { - // Iterate over the rows, filtering for rows that are set - for set_index in rows.iter().enumerate().filter(|a| *a.1).map(|a| a.0) { - // Loop over the selectors previous to this one - for (j, other_selector) in selectors.iter().enumerate().take(i) { - // Look at what selectors are active at the same row - if other_selector.activations[set_index] { - // Mark them as incompatible - exclusion_matrix[i][j] = true; - } + // Loop over the selectors previous to this one + for (j, other_selector) in selectors.iter().enumerate().take(i) { + // Look at what selectors are active at the same row + if rows + .iter() + .zip(other_selector.activations.iter()) + .any(|(l, r)| l & r) + { + // Mark them as incompatible + exclusion_matrix[i][j] = true; } } } @@ -143,7 +145,7 @@ where // Try to find other selectors that can join this one. 'try_selectors: for (j, selector) in selectors.iter().enumerate().skip(i + 1) { - if d == max_degree { + if d + combination.len() == max_degree { // Short circuit; nothing can be added to this // combination. break 'try_selectors; @@ -162,7 +164,7 @@ where } } - // Can the new selector join the combination? Reminder: we subtract + // Can the new selector join the combination? Reminder: we use // selector.max_degree - 1 to omit the influence of the virtual // selector on the degree, as it will be substituted. let new_d = std::cmp::max(d, selector.max_degree - 1); @@ -185,7 +187,13 @@ where let mut assigned_root = F::one(); selector_assignments.extend(combination.into_iter().map(|selector| { - // Compute the expression for substitution + // Compute the expression for substitution. This produces an expression of the + // form + // q * Prod[i = 1..=combination_len, i != assigned_root](i - q) + // + // which is non-zero only on rows where `combination_assignment` is set to + // `assigned_root`. In particular, rows set to 0 correspond to all selectors + // being disabled. let mut expression = query.clone(); let mut root = F::one(); for _ in 0..combination_len { @@ -305,7 +313,11 @@ mod tests { selectors[selector.selector].activations.len(), combination_assignments[selector.combination_index].len() ); - for (&activation, &assignment) in selectors[selector.selector].activations.iter().zip(combination_assignments[selector.combination_index].iter()) { + for (&activation, &assignment) in selectors[selector.selector] + .activations + .iter() + .zip(combination_assignments[selector.combination_index].iter()) + { let eval = selector.expression.evaluate( &|c| c, &|_| panic!("should not occur in returned expressions"),