From 9c32029e9a80dd4526f06b09c88930aea3d3b651 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Fri, 5 Jan 2024 21:06:24 +0000 Subject: [PATCH 1/7] feat: lookups support --- asm_to_pil/src/vm_to_constrained.rs | 5 +- ast/src/parsed/display.rs | 2 +- ast/src/parsed/mod.rs | 1 + ast/src/parsed/visitor.rs | 4 +- bberg/src/lib.rs | 1 + bberg/src/lookup_builder.rs | 358 ++++++++++++++++++++++++ bberg/src/permutation_builder.rs | 35 +-- bberg/src/utils.rs | 40 +++ bberg/src/vm_builder.rs | 18 +- linker/src/lib.rs | 2 +- parser/src/powdr.lalrpop | 2 +- pil_analyzer/src/statement_processor.rs | 4 +- type_check/src/lib.rs | 2 +- 13 files changed, 427 insertions(+), 47 deletions(-) create mode 100644 bberg/src/lookup_builder.rs diff --git a/asm_to_pil/src/vm_to_constrained.rs b/asm_to_pil/src/vm_to_constrained.rs index e21b02d198..d944ce7d8f 100644 --- a/asm_to_pil/src/vm_to_constrained.rs +++ b/asm_to_pil/src/vm_to_constrained.rs @@ -180,6 +180,7 @@ impl ASMPILConverter { self.pil.push(PilStatement::PlookupIdentity( 0, + None, SelectedExpressions { selector: None, expressions: self @@ -417,8 +418,8 @@ impl ASMPILConverter { } } else { match &mut statement { - PilStatement::PermutationIdentity(_, _, left, _) - | PilStatement::PlookupIdentity(_, left, _) => { + PilStatement::PermutationIdentity(_, _attr, left, _) + | PilStatement::PlookupIdentity(_, _attr, left, _) => { assert!( left.selector.is_none(), "LHS selector not supported, could and-combine with instruction flag later." diff --git a/ast/src/parsed/display.rs b/ast/src/parsed/display.rs index 27c435f543..d907697a74 100644 --- a/ast/src/parsed/display.rs +++ b/ast/src/parsed/display.rs @@ -390,7 +390,7 @@ impl Display for PilStatement { write!(f, "{expression} = 0;") } } - PilStatement::PlookupIdentity(_, left, right) => write!(f, "{left} in {right};"), + PilStatement::PlookupIdentity(_, _,left, right) => write!(f, "{left} in {right};"), PilStatement::PermutationIdentity( _, // _, // diff --git a/ast/src/parsed/mod.rs b/ast/src/parsed/mod.rs index 3a588c7c76..1c6bfeafb5 100644 --- a/ast/src/parsed/mod.rs +++ b/ast/src/parsed/mod.rs @@ -37,6 +37,7 @@ pub enum PilStatement { PolynomialIdentity(usize, Option, Expression), PlookupIdentity( usize, + Option, SelectedExpressions>, SelectedExpressions>, ), diff --git a/ast/src/parsed/visitor.rs b/ast/src/parsed/visitor.rs index 831249137b..545162aec3 100644 --- a/ast/src/parsed/visitor.rs +++ b/ast/src/parsed/visitor.rs @@ -192,7 +192,7 @@ impl ExpressionVisitable> for Pi { match self { PilStatement::Expression(_, e) => e.visit_expressions_mut(f, o), - PilStatement::PlookupIdentity(_, left, right) + PilStatement::PlookupIdentity(_,_, left, right) | PilStatement::PermutationIdentity( _, // _, // @@ -234,7 +234,7 @@ impl ExpressionVisitable> for Pi { match self { PilStatement::Expression(_, e) => e.visit_expressions(f, o), - PilStatement::PlookupIdentity(_, left, right) + PilStatement::PlookupIdentity(_,_, left, right) | PilStatement::PermutationIdentity( _, // _, // diff --git a/bberg/src/lib.rs b/bberg/src/lib.rs index 0682b25107..15bf9d0662 100644 --- a/bberg/src/lib.rs +++ b/bberg/src/lib.rs @@ -4,6 +4,7 @@ mod composer_builder; mod file_writer; mod flavor_builder; pub mod permutation_builder; +pub mod lookup_builder; mod prover_builder; mod relation_builder; mod utils; diff --git a/bberg/src/lookup_builder.rs b/bberg/src/lookup_builder.rs new file mode 100644 index 0000000000..0a32352f3b --- /dev/null +++ b/bberg/src/lookup_builder.rs @@ -0,0 +1,358 @@ + +use crate::{file_writer::BBFiles, utils::{create_get_const_entities, create_get_nonconst_entities}}; +use ast::{ + analyzed::{AlgebraicExpression, Analyzed, Identity, IdentityKind}, + parsed::SelectedExpressions, +}; +use itertools::Itertools; +use number::FieldElement; + +use crate::utils::sanitize_name; + +#[derive(Debug)] +/// Lookup +/// +/// Contains the information required to produce a lookup relation +pub struct Lookup { + /// the name given to the inverse helper column + pub attribute: Option, + /// The name of the counts polynomial that stores the number of times a lookup is read + pub counts_poly: String, + /// the left side of the lookup + pub left: LookupSide, + /// the right side of the lookup + pub right: LookupSide, +} + +#[derive(Debug)] +/// PermSide +/// +/// One side of a two sided lookup relationship +pub struct LookupSide { + /// -> Option - the selector for the lookup ( on / off toggle ) + selector: Option, + /// The columns involved in this side of the lookup + cols: Vec, +} + +pub trait LookupBuilder { + /// Takes in an AST and works out what lookup relations are needed + /// Note: returns the name of the inverse columns, such that they can be added to he prover in subsequent steps + fn create_lookup_files( + &self, + name: &str, + analyzed: &Analyzed, + ) -> Vec; +} + +impl LookupBuilder for BBFiles { + fn create_lookup_files( + &self, + project_name: &str, + analyzed: &Analyzed, + ) -> Vec { + let perms: Vec<&Identity>> = analyzed + .identities + .iter() + .filter(|identity| matches!(identity.kind, IdentityKind::Plookup)) + .collect(); + let new_perms = perms + .iter() + .map(|perm| Lookup { + attribute: perm.attribute.clone(), + counts_poly: format!("{}_counts", perm.attribute.clone().unwrap()), + left: get_perm_side(&perm.left), + right: get_perm_side(&perm.right), + }) + .collect_vec(); + + create_lookups(self, project_name, &new_perms); + new_perms + } +} + +/// The attributes of a lookup contain the name of the inverse, we collect all of these to create the inverse column +pub fn get_inverses_from_lookups(lookups: &[Lookup]) -> Vec { + lookups + .iter() + .map(|perm| perm.attribute.clone().unwrap()) + .collect() +} + +pub fn get_counts_from_lookups(lookups: &[Lookup]) -> Vec { + lookups + .iter() + .map(|perm| perm.counts_poly.clone()) + .collect() +} + +/// Write the lookup settings files to disk +fn create_lookups(bb_files: &BBFiles, project_name: &str, lookups: &Vec) { + for lookup in lookups { + let perm_settings = create_lookup_settings_file(lookup); + + let folder = format!("{}/{}", bb_files.rel, project_name); + let file_name = format!( + "{}{}", + lookup.attribute.clone().unwrap_or("NONAME".to_owned()), + ".hpp".to_owned() + ); + bb_files.write_file(&folder, &file_name, &perm_settings); + } +} + +/// All relation types eventually get wrapped in the relation type +/// This function creates the export for the relation type so that it can be added to the flavor +fn create_relation_exporter(lookup_name: &str) -> String { + let settings_name = format!("{}_lookup_settings", lookup_name); + let lookup_export = format!("template using {lookup_name}_relation = GenericLookupRelation<{settings_name}, FF_>;"); + let relation_export = format!("template using {lookup_name} = GenericLookup<{settings_name}, FF_>;"); + + format!( + " + {lookup_export} + {relation_export} + " + ) +} + +fn lookup_settings_includes() -> &'static str { + r#" + #pragma once + + #include "barretenberg/relations/generic_lookup/generic_lookup_relation.hpp" + + #include + #include + "# +} + +fn create_lookup_settings_file(lookup: &Lookup) -> String { + println!("Lookup: {:?}", lookup); + let columns_per_set = lookup.left.cols.len(); + // TODO(md): Throw an error if no attribute is provided for the lookup + // TODO(md): In the future we will need to condense off the back of this - combining those with the same inverse column + let lookup_name = lookup + .attribute + .clone() + .expect("Inverse column name must be provided"); // TODO(md): catch this earlier than here + let counts_poly_name = lookup.counts_poly.to_owned(); + + // NOTE: syntax is not flexible enough to enable the single row case right now :(:(:(:(:)))) + // This also will need to work for both sides of this ! + let lhs_selector = lookup.left.selector.clone().expect("Left hand side selector for lookup required"); // TODO: deal with unwrap + let rhs_selector = lookup.right.selector.clone().expect("Right hand side selector for lookup required"); // TODO: deal with unwrap + let lhs_cols = lookup.left.cols.clone(); + let rhs_cols = lookup.right.cols.clone(); + + assert!(lhs_cols.len() == rhs_cols.len(), "Lookup columns lhs must be the same length as rhs"); + + // 0. The polynomial containing the inverse products -> taken from the attributes + // 1. The polynomial with the counts! + // 2. lhs selector + // 3. rhs selector + // 4.. + columns per set. lhs cols + // 4 + columns per set.. . rhs cols + let mut lookup_entities: Vec = [ + lookup_name.clone(), + counts_poly_name.clone(), + lhs_selector.clone(), + rhs_selector.clone(), // TODO: update this away from the simple example + ] + .to_vec(); + + lookup_entities.extend(lhs_cols); + lookup_entities.extend(rhs_cols); + + // TODO: implement + // NOTE: these are hardcoded as 1 for now until more optimizations are required + let read_terms = 1; + let write_terms = 1; + let lookup_tuple_size = columns_per_set; + + // NOTE: hardcoded until optimizations required + let inverse_degree = 2; + let read_term_degree = 0; + let write_term_degree = 0; + let read_term_types = "{0}"; + let write_term_types = "{0}"; + + let lookup_settings_includes = lookup_settings_includes(); + let inverse_polynomial_is_computed_at_row = create_inverse_computed_at(&lhs_selector, &rhs_selector); + let compute_inverse_exists = create_compute_inverse_exist(&lhs_selector, &rhs_selector); + let const_entities = create_get_const_entities(&lookup_entities); + let nonconst_entities = create_get_nonconst_entities(&lookup_entities); + let relation_exporter = create_relation_exporter(&lookup_name); + + format!( + // TODO: replace with the inverse label name! + " + {lookup_settings_includes} + + namespace proof_system::honk::sumcheck {{ + + /** + * @brief This class contains an example of how to set LookupSettings classes used by the + * GenericLookupRelationImpl class to specify a scaled lookup + * + * @details To create your own lookup: + * 1) Create a copy of this class and rename it + * 2) Update all the values with the ones needed for your permutation + * 3) Update \"DECLARE_LOOKUP_IMPLEMENTATIONS_FOR_ALL_SETTINGS\" and \"DEFINE_LOOKUP_IMPLEMENTATIONS_FOR_ALL_SETTINGS\" to + * include the new settings + * 4) Add the relation with the chosen settings to Relations in the flavor (for example,\"` + * using Relations = std::tuple>;)` + * + */ + class {lookup_name}_lookup_settings {{ + public: + /** + * @brief The number of read terms (how many lookups we perform) in each row + * + */ + static constexpr size_t READ_TERMS = {read_terms}; + /** + * @brief The number of write terms (how many additions to the lookup table we make) in each row + * + */ + static constexpr size_t WRITE_TERMS = {write_terms}; + + /** + * @brief The type of READ_TERM used for each read index (basic and scaled) + * + */ + static constexpr size_t READ_TERM_TYPES[READ_TERMS] = {read_term_types}; + + /** + * @brief They type of WRITE_TERM used for each write index + * + */ + static constexpr size_t WRITE_TERM_TYPES[WRITE_TERMS] = {write_term_types}; + + /** + * @brief How many values represent a single lookup object. This value is used by the automatic read term + * implementation in the relation in case the lookup is a basic or scaled tuple and in the write term if it's a + * basic tuple + * + */ + static constexpr size_t LOOKUP_TUPLE_SIZE = {lookup_tuple_size}; + + /** + * @brief The polynomial degree of the relation telling us if the inverse polynomial value needs to be computed + * + */ + static constexpr size_t INVERSE_EXISTS_POLYNOMIAL_DEGREE = {inverse_degree}; + + /** + * @brief The degree of the read term if implemented arbitrarily. This value is not used by basic and scaled read + * terms, but will cause compilation error if not defined + * + */ + static constexpr size_t READ_TERM_DEGREE = {read_term_degree}; + + /** + * @brief The degree of the write term if implemented arbitrarily. This value is not used by the basic write + * term, but will cause compilation error if not defined + * + */ + + static constexpr size_t WRITE_TERM_DEGREE = {write_term_degree}; + + /** + * @brief If this method returns true on a row of values, then the inverse polynomial exists at this index. + * Otherwise the value needs to be set to zero. + * + * @details If this is true then the lookup takes place in this row + * + */ + {inverse_polynomial_is_computed_at_row} + + /** + * @brief Subprocedure for computing the value deciding if the inverse polynomial value needs to be checked in this + * row + * + * @tparam Accumulator Type specified by the lookup relation + * @tparam AllEntities Values/Univariates of all entities row + * @param in Value/Univariate of all entities at row/edge + * @return Accumulator + */ + {compute_inverse_exists} + + /** + * @brief Get all the entities for the lookup when need to update them + * + * @details The generic structure of this tuple is described in ./generic_lookup_relation.hpp . The following is + description for the current case: + The entities are returned as a tuple of references in the following order (this is for ): + * - The entity/polynomial used to store the product of the inverse values + * - The entity/polynomial that specifies how many times the lookup table entry at this row has been looked up + * - READ_TERMS entities/polynomials that enable individual lookup operations + * - The entity/polynomial that enables adding an entry to the lookup table in this row + * - LOOKUP_TUPLE_SIZE entities/polynomials representing the basic tuple being looked up as the first read term + * - LOOKUP_TUPLE_SIZE entities/polynomials representing the previous accumulators in the second read term + (scaled tuple) + * - LOOKUP_TUPLE_SIZE entities/polynomials representing the shifts in the second read term (scaled tuple) + * - LOOKUP_TUPLE_SIZE entities/polynomials representing the current accumulators in the second read term + (scaled tuple) + * - LOOKUP_TUPLE_SIZE entities/polynomials representing basic tuples added to the table + * + * @return All the entities needed for the lookup + */ + {const_entities} + + /** + * @brief Get all the entities for the lookup when only need to read them + * @details Same as in get_const_entities, but nonconst + * + * @return All the entities needed for the lookup + */ + {nonconst_entities} + }}; + + {relation_exporter} + }} + " + ) +} + +// TODO: make this dynamic such that there can be more than one +fn create_inverse_computed_at(lhs_selector: &String, rhs_selector: &String) -> String { + let lhs_computed_selector = format!("in.{lhs_selector}"); + let rhs_computed_selector = format!("in.{rhs_selector}"); + format!(" + template static inline auto inverse_polynomial_is_computed_at_row(const AllEntities& in) {{ + return ({lhs_computed_selector } == 1 || {rhs_computed_selector} == 1); + }}") +} + +fn create_compute_inverse_exist(lhs_selector: &String, rhs_selector: &String) -> String { + let lhs_computed_selector = format!("in.{lhs_selector}"); + let rhs_computed_selector = format!("in.{rhs_selector}"); + format!(" + template static inline auto inverse_polynomial_is_computed_at_row(const AllEntities& in) {{ + using View = typename Accumulator::View; + const auto is_operation = View({lhs_computed_selector}); + const auto is_table_entry = View({rhs_computed_selector}); + return (is_operation + is_table_entry - is_operation * is_table_entry); + }}") +} + +fn get_perm_side( + def: &SelectedExpressions>, +) -> LookupSide { + let get_name = |expr: &AlgebraicExpression| match expr { + AlgebraicExpression::Reference(a_ref) => sanitize_name(&a_ref.name), + _ => panic!("Expected reference"), + }; + + LookupSide { + selector: def.selector.as_ref().map(|expr| get_name(expr)), + cols: def + .expressions + .iter() + .map(|expr| get_name(expr)) + .collect_vec(), + } +} + \ No newline at end of file diff --git a/bberg/src/permutation_builder.rs b/bberg/src/permutation_builder.rs index 9d501461dd..3dc78532e7 100644 --- a/bberg/src/permutation_builder.rs +++ b/bberg/src/permutation_builder.rs @@ -1,4 +1,4 @@ -use crate::file_writer::BBFiles; +use crate::{file_writer::BBFiles, utils::{create_get_const_entities, create_get_nonconst_entities}}; use ast::{ analyzed::{AlgebraicExpression, Analyzed, Identity, IdentityKind}, parsed::SelectedExpressions, @@ -227,39 +227,6 @@ fn create_inverse_computed_at(inverse_selector: String) -> String { }}") } -fn create_get_const_entities(settings: &[String]) -> String { - let forward = create_forward_as_tuple(settings); - format!( - " - template static inline auto get_const_entities(const AllEntities& in) {{ - {forward} - }} - " - ) -} - -fn create_get_nonconst_entities(settings: &[String]) -> String { - let forward = create_forward_as_tuple(settings); - format!( - " - template static inline auto get_nonconst_entities(AllEntities& in) {{ - {forward} - }} - " - ) -} - -fn create_forward_as_tuple(settings: &[String]) -> String { - let adjusted = settings.iter().map(|col| format!("in.{col}")).join(",\n"); - format!( - " - return std::forward_as_tuple( - {} - ); - ", - adjusted - ) -} fn get_perm_side( def: &SelectedExpressions>, diff --git a/bberg/src/utils.rs b/bberg/src/utils.rs index 676c3fac1d..7883480dbd 100644 --- a/bberg/src/utils.rs +++ b/bberg/src/utils.rs @@ -1,4 +1,5 @@ use number::FieldElement; +use itertools::Itertools; /// Get Relations Imports /// @@ -71,3 +72,42 @@ pub fn flatten(list: &[Vec]) -> Vec { let arr = list.iter().cloned(); arr.into_iter().flatten().collect() } + + +/// Create Forward As Tuple +/// +/// Helper function to create a forward as tuple cpp statement +pub fn create_forward_as_tuple(settings: &[String]) -> String { + let adjusted = settings.iter().map(|col| format!("in.{col}")).join(",\n"); + format!( + " + return std::forward_as_tuple( + {} + ); + ", + adjusted + ) +} + +// TODO: may make sense to move the below around a bit +pub fn create_get_const_entities(settings: &[String]) -> String { + let forward = create_forward_as_tuple(settings); + format!( + " + template static inline auto get_const_entities(const AllEntities& in) {{ + {forward} + }} + " + ) +} + +pub fn create_get_nonconst_entities(settings: &[String]) -> String { + let forward = create_forward_as_tuple(settings); + format!( + " + template static inline auto get_nonconst_entities(AllEntities& in) {{ + {forward} + }} + " + ) +} \ No newline at end of file diff --git a/bberg/src/vm_builder.rs b/bberg/src/vm_builder.rs index 11c4cab7e4..c5139f3af4 100644 --- a/bberg/src/vm_builder.rs +++ b/bberg/src/vm_builder.rs @@ -6,6 +6,10 @@ use crate::circuit_builder::CircuitBuilder; use crate::composer_builder::ComposerBuilder; use crate::file_writer::BBFiles; use crate::flavor_builder::FlavorBuilder; +use crate::lookup_builder::Lookup; +use crate::lookup_builder::LookupBuilder; +use crate::lookup_builder::get_counts_from_lookups; +use crate::lookup_builder::get_inverses_from_lookups; use crate::permutation_builder::get_inverses_from_permutations; use crate::permutation_builder::Permutation; use crate::permutation_builder::PermutationBuilder; @@ -34,6 +38,8 @@ struct ColumnGroups { shifted: Vec, /// fixed + witness + shifted all_cols_with_shifts: Vec, + /// Inverses from lookups and permuations + inverses: Vec, } /// Analyzed to cpp @@ -60,7 +66,7 @@ pub(crate) fn analyzed_to_cpp( // ----------------------- Handle Lookup / Permutation Relation Identities ----------------------- let permutations = bb_files.create_permutation_files(file_name, analyzed); - let inverses = get_inverses_from_permutations(&permutations); + let lookups = bb_files.create_lookup_files(file_name, analyzed); // TODO: hack - this can be removed with some restructuring let shifted_polys: Vec = shifted_polys @@ -78,7 +84,8 @@ pub(crate) fn analyzed_to_cpp( to_be_shifted, shifted, all_cols_with_shifts, - } = get_all_col_names(fixed, witness, &shifted_polys, &permutations); + inverses, + } = get_all_col_names(fixed, witness, &shifted_polys, &permutations, &lookups); bb_files.create_declare_views(file_name, &all_cols_with_shifts); @@ -133,17 +140,21 @@ fn get_all_col_names( witness: &[(String, Vec)], to_be_shifted: &[String], permutations: &[Permutation], + lookups: &[Lookup], ) -> ColumnGroups { // Transformations let sanitize = |(name, _): &(String, Vec)| sanitize_name(name).to_owned(); let append_shift = |name: &String| format!("{}_shift", *name); let perm_inverses = get_inverses_from_permutations(permutations); + let lookup_inverses = get_inverses_from_lookups(lookups); + let counts = get_counts_from_lookups(lookups); // Gather sanitized column names let fixed_names = collect_col(fixed, sanitize); let witness_names = collect_col(witness, sanitize); - let witness_names = flatten(&[witness_names, perm_inverses]); + let inverses = flatten(&[perm_inverses, lookup_inverses, counts]); + let witness_names = flatten(&[witness_names, inverses.clone()]); // Group columns by properties let shifted = transform_map(to_be_shifted, append_shift); @@ -164,5 +175,6 @@ fn get_all_col_names( to_be_shifted: to_be_shifted.to_vec(), shifted, all_cols_with_shifts, + inverses } } diff --git a/linker/src/lib.rs b/linker/src/lib.rs index a106b1a69d..093c357749 100644 --- a/linker/src/lib.rs +++ b/linker/src/lib.rs @@ -101,7 +101,7 @@ pub fn link(graph: PILGraph) -> Result, Vec = { } PlookupIdentity: PilStatement = { - <@L> "in" => PilStatement::PlookupIdentity(<>) + <@L> "in" => PilStatement::PlookupIdentity(<>) } SelectedExpressions: SelectedExpressions> = { diff --git a/pil_analyzer/src/statement_processor.rs b/pil_analyzer/src/statement_processor.rs index 22c081f8e4..5bb02bbd15 100644 --- a/pil_analyzer/src/statement_processor.rs +++ b/pil_analyzer/src/statement_processor.rs @@ -219,10 +219,10 @@ where }, SelectedExpressions::default(), ), - PilStatement::PlookupIdentity(start, key, haystack) => ( + PilStatement::PlookupIdentity(start, attribute, key, haystack) => ( start, IdentityKind::Plookup, - None, + attribute.clone(), self.process_selected_expressions(key), self.process_selected_expressions(haystack), ), diff --git a/type_check/src/lib.rs b/type_check/src/lib.rs index 91d5496bea..45b21bc80a 100644 --- a/type_check/src/lib.rs +++ b/type_check/src/lib.rs @@ -325,7 +325,7 @@ impl TypeChecker { l, _, ) - | ast::parsed::PilStatement::PlookupIdentity(_, l, _) => l + | ast::parsed::PilStatement::PlookupIdentity(_,_, l, _) => l .selector .is_some() .then_some(format!("LHS selector not yet supported in {s}.")), From 82913d65db0419c455b17f49d0b0bbc8a1a4af8c Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Mon, 8 Jan 2024 12:23:06 +0000 Subject: [PATCH 2/7] fix --- bberg/src/circuit_builder.rs | 39 ++++++++++++++++++------------------ bberg/src/lookup_builder.rs | 2 +- bberg/src/vm_builder.rs | 9 +++++---- 3 files changed, 26 insertions(+), 24 deletions(-) diff --git a/bberg/src/circuit_builder.rs b/bberg/src/circuit_builder.rs index f986c65a80..6f90d46032 100644 --- a/bberg/src/circuit_builder.rs +++ b/bberg/src/circuit_builder.rs @@ -28,6 +28,7 @@ fn circuit_hpp_includes(name: &str, relations: &[String], permutations: &[String #include \"barretenberg/ecc/curves/bn254/fr.hpp\" #include \"barretenberg/proof_system/circuit_builder/circuit_builder_base.hpp\" #include \"barretenberg/relations/generic_permutation/generic_permutation_relation.hpp\" + #include \"barretenberg/relations/generic_lookup/generic_lookup_relation.hpp\" #include \"barretenberg/honk/proof_system/logderivative_library.hpp\" #include \"barretenberg/flavor/generated/{name}_flavor.hpp\" @@ -86,12 +87,12 @@ impl CircuitBuilder for BBFiles { relation_name = relation_name ) }; - let check_permutation_transformation = |permutation_name: &String| { + let check_lookup_transformation = |lookup_name: &String| { format!( - "if (!evaluate_permutation.template operator()>(\"{permutation_name}\")) {{ + "if (!evaluate_logderivative.template operator()>(\"{lookup_name}\")) {{ return false; }}", - permutation_name = permutation_name + lookup_name = lookup_name ) }; @@ -100,11 +101,11 @@ impl CircuitBuilder for BBFiles { let all_poly_shifts = map_with_newline(to_be_shifted, all_polys_transformation); let check_circuit_for_each_relation = map_with_newline(relations, check_circuit_transformation); - let check_circuit_for_each_permutation = - map_with_newline(permutations, check_permutation_transformation); + let check_circuit_for_each_lookup = + map_with_newline(permutations, check_lookup_transformation); - let (params, permutation_check_closure) = if !permutations.is_empty() { - (get_params(), get_permutation_check_closure()) + let (params, lookup_check_closure) = if !permutations.is_empty() { + (get_params(), get_lookup_check_closure()) } else { ("", "".to_owned()) }; @@ -166,11 +167,11 @@ class {name}CircuitBuilder {{ {relation_check_closure} - {permutation_check_closure} + {lookup_check_closure} {check_circuit_for_each_relation} - {check_circuit_for_each_permutation} + {check_circuit_for_each_lookup} return true; }} @@ -199,28 +200,28 @@ class {name}CircuitBuilder {{ } } -fn get_permutation_check_closure() -> String { +fn get_lookup_check_closure() -> String { " - const auto evaluate_permutation = [&](const std::string& permutation_name) { + const auto evaluate_logderivative = [&](const std::string& lookup_name) { - // Check the tuple permutation relation + // Check the logderivative relation proof_system::honk::logderivative_library::compute_logderivative_inverse< Flavor, - PermutationSettings>( + LogDerivativeSettings>( polys, params, num_rows); - typename PermutationSettings::SumcheckArrayOfValuesOverSubrelations - permutation_result; + typename LogDerivativeSettings::SumcheckArrayOfValuesOverSubrelations + lookup_result; - for (auto& r : permutation_result) { + for (auto& r : lookup_result) { r = 0; } for (size_t i = 0; i < num_rows; ++i) { - PermutationSettings::accumulate(permutation_result, polys.get_row(i), params, 1); + LogDerivativeSettings::accumulate(lookup_result, polys.get_row(i), params, 1); } - for (auto r : permutation_result) { + for (auto r : lookup_result) { if (r != 0) { - info(\"Tuple \", permutation_name, \" failed.\"); + info(\"Lookup \", lookup_name, \" failed.\"); return false; } } diff --git a/bberg/src/lookup_builder.rs b/bberg/src/lookup_builder.rs index 0a32352f3b..80ac4587ef 100644 --- a/bberg/src/lookup_builder.rs +++ b/bberg/src/lookup_builder.rs @@ -330,7 +330,7 @@ fn create_compute_inverse_exist(lhs_selector: &String, rhs_selector: &String) -> let lhs_computed_selector = format!("in.{lhs_selector}"); let rhs_computed_selector = format!("in.{rhs_selector}"); format!(" - template static inline auto inverse_polynomial_is_computed_at_row(const AllEntities& in) {{ + template static inline auto compute_inverse_exists(const AllEntities& in) {{ using View = typename Accumulator::View; const auto is_operation = View({lhs_computed_selector}); const auto is_table_entry = View({rhs_computed_selector}); diff --git a/bberg/src/vm_builder.rs b/bberg/src/vm_builder.rs index c5139f3af4..d2e3f7ca3e 100644 --- a/bberg/src/vm_builder.rs +++ b/bberg/src/vm_builder.rs @@ -97,6 +97,7 @@ pub(crate) fn analyzed_to_cpp( &all_cols, &to_be_shifted, &all_cols_with_shifts, + ); // ----------------------- Create the flavor file ----------------------- @@ -148,13 +149,13 @@ fn get_all_col_names( let perm_inverses = get_inverses_from_permutations(permutations); let lookup_inverses = get_inverses_from_lookups(lookups); - let counts = get_counts_from_lookups(lookups); + let lookup_counts = get_counts_from_lookups(lookups); // Gather sanitized column names let fixed_names = collect_col(fixed, sanitize); let witness_names = collect_col(witness, sanitize); - let inverses = flatten(&[perm_inverses, lookup_inverses, counts]); - let witness_names = flatten(&[witness_names, inverses.clone()]); + let inverses = flatten(&[perm_inverses, lookup_inverses]); + let witness_names = flatten(&[witness_names, inverses.clone(), lookup_counts]); // Group columns by properties let shifted = transform_map(to_be_shifted, append_shift); @@ -175,6 +176,6 @@ fn get_all_col_names( to_be_shifted: to_be_shifted.to_vec(), shifted, all_cols_with_shifts, - inverses + inverses, } } From 4bc738ebbf2a1d1109f6ed1617015de12b3a22b3 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Mon, 8 Jan 2024 16:00:21 +0000 Subject: [PATCH 3/7] clean --- ast/src/parsed/display.rs | 2 +- ast/src/parsed/visitor.rs | 4 +-- bberg/src/lib.rs | 2 +- bberg/src/lookup_builder.rs | 50 ++++++++++++++++++-------------- bberg/src/permutation_builder.rs | 6 ++-- bberg/src/utils.rs | 7 ++--- bberg/src/vm_builder.rs | 5 ++-- parser/src/lib.rs | 1 + type_check/src/lib.rs | 2 +- 9 files changed, 44 insertions(+), 35 deletions(-) diff --git a/ast/src/parsed/display.rs b/ast/src/parsed/display.rs index d907697a74..8d4168b2f3 100644 --- a/ast/src/parsed/display.rs +++ b/ast/src/parsed/display.rs @@ -390,7 +390,7 @@ impl Display for PilStatement { write!(f, "{expression} = 0;") } } - PilStatement::PlookupIdentity(_, _,left, right) => write!(f, "{left} in {right};"), + PilStatement::PlookupIdentity(_, _, left, right) => write!(f, "{left} in {right};"), PilStatement::PermutationIdentity( _, // _, // diff --git a/ast/src/parsed/visitor.rs b/ast/src/parsed/visitor.rs index 545162aec3..8050399a0f 100644 --- a/ast/src/parsed/visitor.rs +++ b/ast/src/parsed/visitor.rs @@ -192,7 +192,7 @@ impl ExpressionVisitable> for Pi { match self { PilStatement::Expression(_, e) => e.visit_expressions_mut(f, o), - PilStatement::PlookupIdentity(_,_, left, right) + PilStatement::PlookupIdentity(_, _, left, right) | PilStatement::PermutationIdentity( _, // _, // @@ -234,7 +234,7 @@ impl ExpressionVisitable> for Pi { match self { PilStatement::Expression(_, e) => e.visit_expressions(f, o), - PilStatement::PlookupIdentity(_,_, left, right) + PilStatement::PlookupIdentity(_, _, left, right) | PilStatement::PermutationIdentity( _, // _, // diff --git a/bberg/src/lib.rs b/bberg/src/lib.rs index 15bf9d0662..7067a6012a 100644 --- a/bberg/src/lib.rs +++ b/bberg/src/lib.rs @@ -3,8 +3,8 @@ mod circuit_builder; mod composer_builder; mod file_writer; mod flavor_builder; -pub mod permutation_builder; pub mod lookup_builder; +pub mod permutation_builder; mod prover_builder; mod relation_builder; mod utils; diff --git a/bberg/src/lookup_builder.rs b/bberg/src/lookup_builder.rs index 80ac4587ef..050a990a02 100644 --- a/bberg/src/lookup_builder.rs +++ b/bberg/src/lookup_builder.rs @@ -1,5 +1,7 @@ - -use crate::{file_writer::BBFiles, utils::{create_get_const_entities, create_get_nonconst_entities}}; +use crate::{ + file_writer::BBFiles, + utils::{create_get_const_entities, create_get_nonconst_entities}, +}; use ast::{ analyzed::{AlgebraicExpression, Analyzed, Identity, IdentityKind}, parsed::SelectedExpressions, @@ -16,7 +18,7 @@ use crate::utils::sanitize_name; pub struct Lookup { /// the name given to the inverse helper column pub attribute: Option, - /// The name of the counts polynomial that stores the number of times a lookup is read + /// The name of the counts polynomial that stores the number of times a lookup is read pub counts_poly: String, /// the left side of the lookup pub left: LookupSide, @@ -106,7 +108,9 @@ fn create_lookups(bb_files: &BBFiles, project_name: &str, lookups: &Vec) fn create_relation_exporter(lookup_name: &str) -> String { let settings_name = format!("{}_lookup_settings", lookup_name); let lookup_export = format!("template using {lookup_name}_relation = GenericLookupRelation<{settings_name}, FF_>;"); - let relation_export = format!("template using {lookup_name} = GenericLookup<{settings_name}, FF_>;"); + let relation_export = format!( + "template using {lookup_name} = GenericLookup<{settings_name}, FF_>;" + ); format!( " @@ -130,22 +134,31 @@ fn lookup_settings_includes() -> &'static str { fn create_lookup_settings_file(lookup: &Lookup) -> String { println!("Lookup: {:?}", lookup); let columns_per_set = lookup.left.cols.len(); - // TODO(md): Throw an error if no attribute is provided for the lookup - // TODO(md): In the future we will need to condense off the back of this - combining those with the same inverse column let lookup_name = lookup .attribute .clone() - .expect("Inverse column name must be provided"); // TODO(md): catch this earlier than here + .expect("Inverse column name must be provided within lookup attribute - #[]"); let counts_poly_name = lookup.counts_poly.to_owned(); // NOTE: syntax is not flexible enough to enable the single row case right now :(:(:(:(:)))) // This also will need to work for both sides of this ! - let lhs_selector = lookup.left.selector.clone().expect("Left hand side selector for lookup required"); // TODO: deal with unwrap - let rhs_selector = lookup.right.selector.clone().expect("Right hand side selector for lookup required"); // TODO: deal with unwrap + let lhs_selector = lookup + .left + .selector + .clone() + .expect("Left hand side selector for lookup required"); + let rhs_selector = lookup + .right + .selector + .clone() + .expect("Right hand side selector for lookup required"); let lhs_cols = lookup.left.cols.clone(); let rhs_cols = lookup.right.cols.clone(); - assert!(lhs_cols.len() == rhs_cols.len(), "Lookup columns lhs must be the same length as rhs"); + assert!( + lhs_cols.len() == rhs_cols.len(), + "Lookup columns lhs must be the same length as rhs" + ); // 0. The polynomial containing the inverse products -> taken from the attributes // 1. The polynomial with the counts! @@ -157,35 +170,34 @@ fn create_lookup_settings_file(lookup: &Lookup) -> String { lookup_name.clone(), counts_poly_name.clone(), lhs_selector.clone(), - rhs_selector.clone(), // TODO: update this away from the simple example + rhs_selector.clone(), ] .to_vec(); lookup_entities.extend(lhs_cols); lookup_entities.extend(rhs_cols); - // TODO: implement // NOTE: these are hardcoded as 1 for now until more optimizations are required let read_terms = 1; let write_terms = 1; let lookup_tuple_size = columns_per_set; // NOTE: hardcoded until optimizations required - let inverse_degree = 2; + let inverse_degree = 2; let read_term_degree = 0; let write_term_degree = 0; let read_term_types = "{0}"; let write_term_types = "{0}"; let lookup_settings_includes = lookup_settings_includes(); - let inverse_polynomial_is_computed_at_row = create_inverse_computed_at(&lhs_selector, &rhs_selector); - let compute_inverse_exists = create_compute_inverse_exist(&lhs_selector, &rhs_selector); + let inverse_polynomial_is_computed_at_row = + create_inverse_computed_at(&lhs_selector, &rhs_selector); + let compute_inverse_exists = create_compute_inverse_exist(&lhs_selector, &rhs_selector); let const_entities = create_get_const_entities(&lookup_entities); let nonconst_entities = create_get_nonconst_entities(&lookup_entities); let relation_exporter = create_relation_exporter(&lookup_name); format!( - // TODO: replace with the inverse label name! " {lookup_settings_includes} @@ -316,7 +328,6 @@ fn create_lookup_settings_file(lookup: &Lookup) -> String { ) } -// TODO: make this dynamic such that there can be more than one fn create_inverse_computed_at(lhs_selector: &String, rhs_selector: &String) -> String { let lhs_computed_selector = format!("in.{lhs_selector}"); let rhs_computed_selector = format!("in.{rhs_selector}"); @@ -338,9 +349,7 @@ fn create_compute_inverse_exist(lhs_selector: &String, rhs_selector: &String) -> }}") } -fn get_perm_side( - def: &SelectedExpressions>, -) -> LookupSide { +fn get_perm_side(def: &SelectedExpressions>) -> LookupSide { let get_name = |expr: &AlgebraicExpression| match expr { AlgebraicExpression::Reference(a_ref) => sanitize_name(&a_ref.name), _ => panic!("Expected reference"), @@ -355,4 +364,3 @@ fn get_perm_side( .collect_vec(), } } - \ No newline at end of file diff --git a/bberg/src/permutation_builder.rs b/bberg/src/permutation_builder.rs index 3dc78532e7..f64825a03e 100644 --- a/bberg/src/permutation_builder.rs +++ b/bberg/src/permutation_builder.rs @@ -1,4 +1,7 @@ -use crate::{file_writer::BBFiles, utils::{create_get_const_entities, create_get_nonconst_entities}}; +use crate::{ + file_writer::BBFiles, + utils::{create_get_const_entities, create_get_nonconst_entities}, +}; use ast::{ analyzed::{AlgebraicExpression, Analyzed, Identity, IdentityKind}, parsed::SelectedExpressions, @@ -227,7 +230,6 @@ fn create_inverse_computed_at(inverse_selector: String) -> String { }}") } - fn get_perm_side( def: &SelectedExpressions>, ) -> PermutationSide { diff --git a/bberg/src/utils.rs b/bberg/src/utils.rs index 7883480dbd..ed6f72815a 100644 --- a/bberg/src/utils.rs +++ b/bberg/src/utils.rs @@ -1,5 +1,5 @@ -use number::FieldElement; use itertools::Itertools; +use number::FieldElement; /// Get Relations Imports /// @@ -73,9 +73,8 @@ pub fn flatten(list: &[Vec]) -> Vec { arr.into_iter().flatten().collect() } - /// Create Forward As Tuple -/// +/// /// Helper function to create a forward as tuple cpp statement pub fn create_forward_as_tuple(settings: &[String]) -> String { let adjusted = settings.iter().map(|col| format!("in.{col}")).join(",\n"); @@ -110,4 +109,4 @@ pub fn create_get_nonconst_entities(settings: &[String]) -> String { }} " ) -} \ No newline at end of file +} diff --git a/bberg/src/vm_builder.rs b/bberg/src/vm_builder.rs index d2e3f7ca3e..9f8380bc13 100644 --- a/bberg/src/vm_builder.rs +++ b/bberg/src/vm_builder.rs @@ -6,10 +6,10 @@ use crate::circuit_builder::CircuitBuilder; use crate::composer_builder::ComposerBuilder; use crate::file_writer::BBFiles; use crate::flavor_builder::FlavorBuilder; -use crate::lookup_builder::Lookup; -use crate::lookup_builder::LookupBuilder; use crate::lookup_builder::get_counts_from_lookups; use crate::lookup_builder::get_inverses_from_lookups; +use crate::lookup_builder::Lookup; +use crate::lookup_builder::LookupBuilder; use crate::permutation_builder::get_inverses_from_permutations; use crate::permutation_builder::Permutation; use crate::permutation_builder::PermutationBuilder; @@ -97,7 +97,6 @@ pub(crate) fn analyzed_to_cpp( &all_cols, &to_be_shifted, &all_cols_with_shifts, - ); // ----------------------- Create the flavor file ----------------------- diff --git a/parser/src/lib.rs b/parser/src/lib.rs index 0909e43a4c..340d143de9 100644 --- a/parser/src/lib.rs +++ b/parser/src/lib.rs @@ -98,6 +98,7 @@ mod test { parsed, PILFile(vec![PilStatement::PlookupIdentity( 0, + None, SelectedExpressions { selector: None, expressions: vec![direct_reference("f")] diff --git a/type_check/src/lib.rs b/type_check/src/lib.rs index 45b21bc80a..547d42cd1d 100644 --- a/type_check/src/lib.rs +++ b/type_check/src/lib.rs @@ -325,7 +325,7 @@ impl TypeChecker { l, _, ) - | ast::parsed::PilStatement::PlookupIdentity(_,_, l, _) => l + | ast::parsed::PilStatement::PlookupIdentity(_, _, l, _) => l .selector .is_some() .then_some(format!("LHS selector not yet supported in {s}.")), From 5dfcc7253fbe875c89d721f29fb5e4a3282af6ee Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Tue, 9 Jan 2024 16:14:39 +0000 Subject: [PATCH 4/7] chore: renamings --- bberg/src/circuit_builder.rs | 3 +-- bberg/src/lookup_builder.rs | 34 +++++++++++++++++----------------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/bberg/src/circuit_builder.rs b/bberg/src/circuit_builder.rs index 6f90d46032..a7f393c3dd 100644 --- a/bberg/src/circuit_builder.rs +++ b/bberg/src/circuit_builder.rs @@ -91,8 +91,7 @@ impl CircuitBuilder for BBFiles { format!( "if (!evaluate_logderivative.template operator()>(\"{lookup_name}\")) {{ return false; - }}", - lookup_name = lookup_name + }}" ) }; diff --git a/bberg/src/lookup_builder.rs b/bberg/src/lookup_builder.rs index 050a990a02..cb9b9f8e3a 100644 --- a/bberg/src/lookup_builder.rs +++ b/bberg/src/lookup_builder.rs @@ -27,7 +27,7 @@ pub struct Lookup { } #[derive(Debug)] -/// PermSide +/// LookupSide /// /// One side of a two sided lookup relationship pub struct LookupSide { @@ -37,6 +37,7 @@ pub struct LookupSide { cols: Vec, } + pub trait LookupBuilder { /// Takes in an AST and works out what lookup relations are needed /// Note: returns the name of the inverse columns, such that they can be added to he prover in subsequent steps @@ -53,23 +54,23 @@ impl LookupBuilder for BBFiles { project_name: &str, analyzed: &Analyzed, ) -> Vec { - let perms: Vec<&Identity>> = analyzed + let lookups: Vec<&Identity>> = analyzed .identities .iter() .filter(|identity| matches!(identity.kind, IdentityKind::Plookup)) .collect(); - let new_perms = perms + let new_lookups = lookups .iter() - .map(|perm| Lookup { - attribute: perm.attribute.clone(), - counts_poly: format!("{}_counts", perm.attribute.clone().unwrap()), - left: get_perm_side(&perm.left), - right: get_perm_side(&perm.right), + .map(|lookup| Lookup { + attribute: lookup.attribute.clone(), + counts_poly: format!("{}_counts", lookup.attribute.clone().unwrap()), + left: get_lookup_side(&lookup.left), + right: get_lookup_side(&lookup.right), }) .collect_vec(); - create_lookups(self, project_name, &new_perms); - new_perms + create_lookups(self, project_name, &new_lookups); + new_lookups } } @@ -77,21 +78,21 @@ impl LookupBuilder for BBFiles { pub fn get_inverses_from_lookups(lookups: &[Lookup]) -> Vec { lookups .iter() - .map(|perm| perm.attribute.clone().unwrap()) + .map(|lookup| lookup.attribute.clone().unwrap()) .collect() } pub fn get_counts_from_lookups(lookups: &[Lookup]) -> Vec { lookups .iter() - .map(|perm| perm.counts_poly.clone()) + .map(|lookup| lookup.counts_poly.clone()) .collect() } /// Write the lookup settings files to disk fn create_lookups(bb_files: &BBFiles, project_name: &str, lookups: &Vec) { for lookup in lookups { - let perm_settings = create_lookup_settings_file(lookup); + let lookup_settings = create_lookup_settings_file(lookup); let folder = format!("{}/{}", bb_files.rel, project_name); let file_name = format!( @@ -99,7 +100,7 @@ fn create_lookups(bb_files: &BBFiles, project_name: &str, lookups: &Vec) lookup.attribute.clone().unwrap_or("NONAME".to_owned()), ".hpp".to_owned() ); - bb_files.write_file(&folder, &file_name, &perm_settings); + bb_files.write_file(&folder, &file_name, &lookup_settings); } } @@ -132,7 +133,6 @@ fn lookup_settings_includes() -> &'static str { } fn create_lookup_settings_file(lookup: &Lookup) -> String { - println!("Lookup: {:?}", lookup); let columns_per_set = lookup.left.cols.len(); let lookup_name = lookup .attribute @@ -209,7 +209,7 @@ fn create_lookup_settings_file(lookup: &Lookup) -> String { * * @details To create your own lookup: * 1) Create a copy of this class and rename it - * 2) Update all the values with the ones needed for your permutation + * 2) Update all the values with the ones needed for your lookuputation * 3) Update \"DECLARE_LOOKUP_IMPLEMENTATIONS_FOR_ALL_SETTINGS\" and \"DEFINE_LOOKUP_IMPLEMENTATIONS_FOR_ALL_SETTINGS\" to * include the new settings * 4) Add the relation with the chosen settings to Relations in the flavor (for example,\"` @@ -349,7 +349,7 @@ fn create_compute_inverse_exist(lhs_selector: &String, rhs_selector: &String) -> }}") } -fn get_perm_side(def: &SelectedExpressions>) -> LookupSide { +fn get_lookup_side(def: &SelectedExpressions>) -> LookupSide { let get_name = |expr: &AlgebraicExpression| match expr { AlgebraicExpression::Reference(a_ref) => sanitize_name(&a_ref.name), _ => panic!("Expected reference"), From e5e49a123cee2443fc6b74a47dca91860595c72e Mon Sep 17 00:00:00 2001 From: Maddiaa <47148561+Maddiaa0@users.noreply.github.com> Date: Tue, 9 Jan 2024 16:22:16 +0000 Subject: [PATCH 5/7] Apply suggestions from code review Co-authored-by: Jean M <132435771+jeanmon@users.noreply.github.com> --- bberg/src/lookup_builder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bberg/src/lookup_builder.rs b/bberg/src/lookup_builder.rs index cb9b9f8e3a..98caad86d9 100644 --- a/bberg/src/lookup_builder.rs +++ b/bberg/src/lookup_builder.rs @@ -40,7 +40,7 @@ pub struct LookupSide { pub trait LookupBuilder { /// Takes in an AST and works out what lookup relations are needed - /// Note: returns the name of the inverse columns, such that they can be added to he prover in subsequent steps + /// Note: returns the name of the inverse columns, such that they can be added to the prover in subsequent steps fn create_lookup_files( &self, name: &str, @@ -314,7 +314,7 @@ fn create_lookup_settings_file(lookup: &Lookup) -> String { {const_entities} /** - * @brief Get all the entities for the lookup when only need to read them + * @brief Get all the entities for the lookup when we only need to read them * @details Same as in get_const_entities, but nonconst * * @return All the entities needed for the lookup From 8fe6cb3af1b863bcf085e3981d395a07a2e6a83b Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Tue, 9 Jan 2024 16:22:24 +0000 Subject: [PATCH 6/7] add notes --- bberg/src/lookup_builder.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bberg/src/lookup_builder.rs b/bberg/src/lookup_builder.rs index cb9b9f8e3a..a86b75f05c 100644 --- a/bberg/src/lookup_builder.rs +++ b/bberg/src/lookup_builder.rs @@ -15,6 +15,8 @@ use crate::utils::sanitize_name; /// Lookup /// /// Contains the information required to produce a lookup relation +/// Lookup object and lookup side object are very similar in structure, however they are duplicated for +/// readability. pub struct Lookup { /// the name given to the inverse helper column pub attribute: Option, @@ -140,8 +142,9 @@ fn create_lookup_settings_file(lookup: &Lookup) -> String { .expect("Inverse column name must be provided within lookup attribute - #[]"); let counts_poly_name = lookup.counts_poly.to_owned(); - // NOTE: syntax is not flexible enough to enable the single row case right now :(:(:(:(:)))) - // This also will need to work for both sides of this ! + // NOTE: https://github.com/AztecProtocol/aztec-packages/issues/3879 + // Settings are not flexible enough to combine inverses + let lhs_selector = lookup .left .selector From 30fd4f78d0bd595fc71b0f00d1d57bfa19d3efea Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Tue, 9 Jan 2024 16:24:01 +0000 Subject: [PATCH 7/7] fmt --- bberg/src/lookup_builder.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bberg/src/lookup_builder.rs b/bberg/src/lookup_builder.rs index 6d9c486787..490ad104d2 100644 --- a/bberg/src/lookup_builder.rs +++ b/bberg/src/lookup_builder.rs @@ -15,7 +15,7 @@ use crate::utils::sanitize_name; /// Lookup /// /// Contains the information required to produce a lookup relation -/// Lookup object and lookup side object are very similar in structure, however they are duplicated for +/// Lookup object and lookup side object are very similar in structure, however they are duplicated for /// readability. pub struct Lookup { /// the name given to the inverse helper column @@ -39,7 +39,6 @@ pub struct LookupSide { cols: Vec, } - pub trait LookupBuilder { /// Takes in an AST and works out what lookup relations are needed /// Note: returns the name of the inverse columns, such that they can be added to the prover in subsequent steps @@ -352,7 +351,9 @@ fn create_compute_inverse_exist(lhs_selector: &String, rhs_selector: &String) -> }}") } -fn get_lookup_side(def: &SelectedExpressions>) -> LookupSide { +fn get_lookup_side( + def: &SelectedExpressions>, +) -> LookupSide { let get_name = |expr: &AlgebraicExpression| match expr { AlgebraicExpression::Reference(a_ref) => sanitize_name(&a_ref.name), _ => panic!("Expected reference"),