From 0731fec251bb8c5618e342a4b8342d638b217cd0 Mon Sep 17 00:00:00 2001 From: Matthias Goergens Date: Mon, 15 Apr 2024 17:02:48 +0800 Subject: [PATCH] Simplify types: remove option from CTL filters CTL filters can already express perfectly well the behaviour that we gave for `None`. No need to complicated anything. Plus some random lint fixes that clippy demanded.. --- plonky2/src/gates/lookup.rs | 1 - plonky2/src/gates/lookup_table.rs | 1 - plonky2/src/plonk/copy_constraint.rs | 1 + starky/src/cross_table_lookup.rs | 58 ++++++----------------- starky/src/lookup.rs | 69 ++++++++++------------------ starky/src/permutation_stark.rs | 2 +- 6 files changed, 40 insertions(+), 92 deletions(-) diff --git a/plonky2/src/gates/lookup.rs b/plonky2/src/gates/lookup.rs index 23a0fd87..49822170 100644 --- a/plonky2/src/gates/lookup.rs +++ b/plonky2/src/gates/lookup.rs @@ -5,7 +5,6 @@ use alloc::{ vec, vec::Vec, }; -use core::usize; use itertools::Itertools; use keccak_hash::keccak; diff --git a/plonky2/src/gates/lookup_table.rs b/plonky2/src/gates/lookup_table.rs index 9a4d08c8..28f47c4d 100644 --- a/plonky2/src/gates/lookup_table.rs +++ b/plonky2/src/gates/lookup_table.rs @@ -6,7 +6,6 @@ use alloc::{ vec, vec::Vec, }; -use core::usize; #[cfg(feature = "std")] use std::sync::Arc; diff --git a/plonky2/src/plonk/copy_constraint.rs b/plonky2/src/plonk/copy_constraint.rs index 309f207d..58fb1e89 100644 --- a/plonky2/src/plonk/copy_constraint.rs +++ b/plonky2/src/plonk/copy_constraint.rs @@ -7,6 +7,7 @@ use crate::iop::target::Target; #[derive(Debug)] pub struct CopyConstraint { pub pair: (Target, Target), + #[allow(dead_code)] pub name: String, } diff --git a/starky/src/cross_table_lookup.rs b/starky/src/cross_table_lookup.rs index da50c24c..d0b020a6 100644 --- a/starky/src/cross_table_lookup.rs +++ b/starky/src/cross_table_lookup.rs @@ -67,12 +67,12 @@ pub type TableIdx = usize; pub struct TableWithColumns { table: TableIdx, columns: Vec>, - filter: Option>, + filter: Filter, } impl TableWithColumns { /// Generates a new `TableWithColumns` given a `table` index, a linear combination of columns `columns` and a `filter`. - pub fn new(table: TableIdx, columns: Vec>, filter: Option>) -> Self { + pub fn new(table: TableIdx, columns: Vec>, filter: Filter) -> Self { Self { table, columns, @@ -163,7 +163,7 @@ pub struct CtlZData<'a, F: Field> { pub(crate) columns: Vec<&'a [Column]>, /// Vector of filter columns for the current table. /// Each filter evaluates to either 1 or 0. - pub(crate) filter: Vec>>, + pub(crate) filter: Vec>, } impl<'a, F: Field> CtlZData<'a, F> { @@ -173,7 +173,7 @@ impl<'a, F: Field> CtlZData<'a, F> { z: PolynomialValues, challenge: GrandProductChallenge, columns: Vec<&'a [Column]>, - filter: Vec>>, + filter: Vec>, ) -> Self { Self { helper_columns, @@ -404,7 +404,7 @@ fn ctl_helper_zs_cols( .map(|(table, group)| { let columns_filters = group .map(|table| (&table.columns[..], &table.filter)) - .collect::], &Option>)>>(); + .collect::], &Filter)>>(); ( table, partial_sums( @@ -484,7 +484,7 @@ where /// Column linear combinations of the `CrossTableLookup`s. pub(crate) columns: Vec<&'a [Column]>, /// Filter that evaluates to either 1 or 0. - pub(crate) filter: Vec>>, + pub(crate) filter: Vec>, } impl<'a, F: RichField + Extendable, const D: usize> @@ -682,16 +682,8 @@ pub(crate) fn eval_cross_table_lookup_checks { /// Column linear combinations of the `CrossTableLookup`s. pub(crate) columns: Vec>>, /// Filter that evaluates to either 1 or 0. - pub(crate) filter: Vec>>, + pub(crate) filter: Vec>, } impl<'a, F: Field, const D: usize> CtlCheckVarsTarget { @@ -856,8 +844,6 @@ pub(crate) fn eval_cross_table_lookup_checks_circuit< let local_values = vars.get_local_values(); let next_values = vars.get_next_values(); - let one = builder.one_extension(); - for lookup_vars in ctl_vars { let CtlCheckVarsTarget { helper_columns, @@ -906,16 +892,8 @@ pub(crate) fn eval_cross_table_lookup_checks_circuit< let combin0 = challenges.combine_circuit(builder, &evals[0]); let combin1 = challenges.combine_circuit(builder, &evals[1]); - let f0 = if let Some(filter0) = &filter[0] { - filter0.eval_filter_circuit(builder, local_values, next_values) - } else { - one - }; - let f1 = if let Some(filter1) = &filter[1] { - filter1.eval_filter_circuit(builder, local_values, next_values) - } else { - one - }; + let f0 = filter[0].eval_filter_circuit(builder, local_values, next_values); + let f1 = filter[1].eval_filter_circuit(builder, local_values, next_values); let combined = builder.mul_sub_extension(combin1, *local_z, f1); let combined = builder.mul_extension(combined, combin0); @@ -928,11 +906,7 @@ pub(crate) fn eval_cross_table_lookup_checks_circuit< consumer.constraint_last_row(builder, constr); } else { let combin0 = challenges.combine_circuit(builder, &evals[0]); - let f0 = if let Some(filter0) = &filter[0] { - filter0.eval_filter_circuit(builder, local_values, next_values) - } else { - one - }; + let f0 = filter[0].eval_filter_circuit(builder, local_values, next_values); let constr = builder.mul_sub_extension(combin0, *local_z, f0); consumer.constraint_last_row(builder, constr); @@ -1121,11 +1095,7 @@ pub mod debug_utils { ) { let trace = &trace_poly_values[table.table]; for i in 0..trace[0].len() { - let filter = if let Some(combin) = &table.filter { - combin.eval_table(trace, i) - } else { - F::ONE - }; + let filter = table.filter.eval_table(trace, i); if filter.is_one() { let row = table .columns diff --git a/starky/src/lookup.rs b/starky/src/lookup.rs index 16383cd6..f63c4bd7 100644 --- a/starky/src/lookup.rs +++ b/starky/src/lookup.rs @@ -39,6 +39,16 @@ pub struct Filter { constants: Vec>, } +/// The default filter is always on. +impl Default for Filter { + fn default() -> Self { + Self { + products: vec![], + constants: vec![Column::constant(F::ONE)], + } + } +} + impl Filter { /// Returns a filter from the provided `products` and `constants` vectors. pub fn new(products: Vec<(Column, Column)>, constants: Vec>) -> Self { @@ -396,7 +406,7 @@ impl Column { } } -pub(crate) type ColumnFilter<'a, F> = (&'a [Column], &'a Option>); +pub(crate) type ColumnFilter<'a, F> = (&'a [Column], &'a Filter); /// A [`Lookup`] defines a set of `columns`` whose values should appear in a /// `table_column` (i.e. the lookup table associated to these looking columns), @@ -423,7 +433,7 @@ pub struct Lookup { /// Columns to filter some elements. There is at most one filter /// column per column to lookup. - pub filter_columns: Vec>>, + pub filter_columns: Vec>, } impl Lookup { @@ -650,7 +660,7 @@ pub(crate) fn lookup_helper_columns( /// Given data associated to a lookup, check the associated helper polynomials. pub(crate) fn eval_helper_columns( - filter: &[Option>], + filter: &[Filter], columns: &[Vec

], local_values: &[P], next_values: &[P], @@ -674,26 +684,14 @@ pub(crate) fn eval_helper_columns( let combin0 = challenges.combine(&chunk[0]); let combin1 = challenges.combine(chunk[1].iter()); - let f0 = if let Some(filter0) = &fs[0] { - filter0.eval_filter(local_values, next_values) - } else { - P::ONES - }; - let f1 = if let Some(filter1) = &fs[1] { - filter1.eval_filter(local_values, next_values) - } else { - P::ONES - }; + let f0 = fs[0].eval_filter(local_values, next_values); + let f1 = fs[1].eval_filter(local_values, next_values); consumer.constraint(combin1 * combin0 * h - f0 * combin1 - f1 * combin0); } 1 => { let combin = challenges.combine(&chunk[0]); - let f0 = if let Some(filter1) = &fs[0] { - filter1.eval_filter(local_values, next_values) - } else { - P::ONES - }; + let f0 = fs[0].eval_filter(local_values, next_values); consumer.constraint(combin * h - f0); } @@ -707,7 +705,7 @@ pub(crate) fn eval_helper_columns( /// Given data associated to a lookup (either a CTL or a range-check), check the associated helper polynomials. pub(crate) fn eval_helper_columns_circuit, const D: usize>( builder: &mut CircuitBuilder, - filter: &[Option>], + filter: &[Filter], columns: &[Vec>], local_values: &[ExtensionTarget], next_values: &[ExtensionTarget], @@ -722,22 +720,13 @@ pub(crate) fn eval_helper_columns_circuit, const D: .chunks(chunk_size) .zip(filter.chunks(chunk_size).zip(helper_columns)) { - let one = builder.one_extension(); match chunk.len() { 2 => { let combin0 = challenges.combine_circuit(builder, &chunk[0]); let combin1 = challenges.combine_circuit(builder, &chunk[1]); - let f0 = if let Some(filter0) = &fs[0] { - filter0.eval_filter_circuit(builder, local_values, next_values) - } else { - one - }; - let f1 = if let Some(filter1) = &fs[1] { - filter1.eval_filter_circuit(builder, local_values, next_values) - } else { - one - }; + let f0 = fs[0].eval_filter_circuit(builder, local_values, next_values); + let f1 = fs[1].eval_filter_circuit(builder, local_values, next_values); let constr = builder.mul_sub_extension(combin0, h, f0); let constr = builder.mul_extension(constr, combin1); @@ -748,11 +737,7 @@ pub(crate) fn eval_helper_columns_circuit, const D: } 1 => { let combin = challenges.combine_circuit(builder, &chunk[0]); - let f0 = if let Some(filter1) = &fs[0] { - filter1.eval_filter_circuit(builder, local_values, next_values) - } else { - one - }; + let f0 = fs[0].eval_filter_circuit(builder, local_values, next_values); let constr = builder.mul_sub_extension(combin, h, f0); consumer.constraint(builder, constr); } @@ -788,13 +773,10 @@ pub(crate) fn get_helper_cols( let mut filter_col = Vec::with_capacity(degree); let first_combined = (0..degree) .map(|d| { - let f = if let Some(filter) = first_filter { - let f = filter.eval_table(trace, d); + let f = { + let f = first_filter.eval_table(trace, d); filter_col.push(f); f - } else { - filter_col.push(F::ONE); - F::ONE }; if f.is_one() { let evals = first_col @@ -821,13 +803,10 @@ pub(crate) fn get_helper_cols( let mut filter_col = Vec::with_capacity(degree); let mut combined = (0..degree) .map(|d| { - let f = if let Some(filter) = filt { - let f = filter.eval_table(trace, d); + let f = { + let f = filt.eval_table(trace, d); filter_col.push(f); f - } else { - filter_col.push(F::ONE); - F::ONE }; if f.is_one() { let evals = col diff --git a/starky/src/permutation_stark.rs b/starky/src/permutation_stark.rs index bddf4202..62290b65 100644 --- a/starky/src/permutation_stark.rs +++ b/starky/src/permutation_stark.rs @@ -72,7 +72,7 @@ impl, const D: usize> Stark for PermutationSt columns: vec![Column::single(0)], table_column: Column::single(1), frequencies_column: Column::single(2), - filter_columns: vec![None; 1], + filter_columns: vec![Default::default()], }] }