Bug fix

powdr-labs · Feb 8, 2024 · f68b268 · f68b268
1 parent 5e16a94
commit f68b268
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 43 deletions.
diff --git a/halo2/src/circuit_builder.rs b/halo2/src/circuit_builder.rs
@@ -57,17 +57,28 @@ pub(crate) fn analyzed_to_plaf<T: FieldElement>(
 
     let query = |column, rotation| Expr::Var(PlonkVar::Query(ColumnQuery { column, rotation }));
 
-    let mut cd = CircuitData::from(analyzed, fixed.to_owned());
+    let mut cd = CircuitData::from(analyzed, fixed);
+
+    println!("Fixed len: {}", fixed.len());
+
+    let mut fixed_names = fixed
+        .iter()
+        .map(|(name, _)| name.clone())
+        .collect::<Vec<_>>();
+    let mut fixed = fixed
+        .iter()
+        .map(|(_, column)| convert_column(column))
+        .collect::<Vec<_>>();
 
     let num_rows: usize = analyzed.degree() as usize + 1;
 
     // Append __enable fixed column
-    let q_enable = query(
-        cd.insert_constant(
-            "__enable",
-            itertools::repeat_n(T::from(1), num_rows - 1).chain(std::iter::once(T::from(0))),
-        ),
-        0,
+    let q_enable = query(cd.insert_constant("__enable"), 0);
+    fixed_names.push("__enable".to_string());
+    fixed.push(
+        itertools::repeat_n(Some(BigUint::from(1u8)), num_rows - 1)
+            .chain(std::iter::once(Some(BigUint::from(0u8))))
+            .collect::<Vec<_>>(),
     );
 
     let mut lookups = vec![];
@@ -84,10 +95,9 @@ pub(crate) fn analyzed_to_plaf<T: FieldElement>(
     // build Plaf columns -------------------------------------------------
 
     let columns = Columns {
-        fixed: cd
-            .fixed
-            .iter()
-            .map(|(name, _)| ColumnFixed::new(name.to_string()))
+        fixed: fixed_names
+            .into_iter()
+            .map(|name| ColumnFixed::new(name))
             .collect(),
         witness: wit_columns,
         public: vec![ColumnPublic::new("public".to_string())],
@@ -97,7 +107,7 @@ pub(crate) fn analyzed_to_plaf<T: FieldElement>(
 
     let info = Info {
         p: T::modulus().to_arbitrary_integer(),
-        num_rows: cd.len(),
+        num_rows,
         challenges: vec![],
     };
 
@@ -174,12 +184,6 @@ pub(crate) fn analyzed_to_plaf<T: FieldElement>(
 
     // build Plaf fixed. -------------------------------------------------------------------------
 
-    let fixed: Vec<Vec<_>> = cd
-        .fixed
-        .iter()
-        .map(|(_, column)| convert_column(column))
-        .collect();
-
     Plaf {
         info,
         columns,
@@ -192,7 +196,7 @@ pub(crate) fn analyzed_to_plaf<T: FieldElement>(
     }
 }
 
-fn copy_constraints<T: FieldElement>(pil: &Analyzed<T>, cd: &CircuitData<T>) -> Vec<CopyC> {
+fn copy_constraints<T: FieldElement>(pil: &Analyzed<T>, cd: &CircuitData) -> Vec<CopyC> {
     let mut copies = vec![];
 
     // Enforce publics by copy-constraining to cells in the instance column.
@@ -335,7 +339,7 @@ pub(crate) fn analyzed_to_circuit_with_witness<T: FieldElement>(
     )
 }
 
-fn expression_2_expr<T: FieldElement>(cd: &CircuitData<T>, expr: &Expression<T>) -> Expr<PlonkVar> {
+fn expression_2_expr<T: FieldElement>(cd: &CircuitData, expr: &Expression<T>) -> Expr<PlonkVar> {
     match expr {
         Expression::Number(n) => Expr::Const(n.to_arbitrary_integer()),
         Expression::Reference(polyref) => {

diff --git a/halo2/src/circuit_data.rs b/halo2/src/circuit_data.rs
@@ -7,14 +7,14 @@ use polyexen::expr::{Column, ColumnKind};
 use powdr_ast::analyzed::Analyzed;
 use powdr_number::{AbstractNumberType, FieldElement};
 
-pub(crate) struct CircuitData<T> {
-    pub(crate) fixed: Vec<(String, Vec<T>)>,
+pub(crate) struct CircuitData {
     pub(crate) public_column: Column,
     pub columns: HashMap<String, Column>,
+    fixed_id_counter: usize,
 }
 
-impl<'a, T: FieldElement> CircuitData<T> {
-    pub fn from(pil: &Analyzed<T>, fixed: Vec<(String, Vec<T>)>) -> Self {
+impl<'a> CircuitData {
+    pub fn from<T: FieldElement>(pil: &Analyzed<T>, fixed: &[(String, Vec<T>)]) -> Self {
         let const_cols = fixed.iter().enumerate().map(|(index, (name, _))| {
             (
                 name.to_string(),
@@ -47,9 +47,9 @@ impl<'a, T: FieldElement> CircuitData<T> {
         };
 
         Self {
-            fixed,
             columns,
             public_column,
+            fixed_id_counter: fixed.len(),
         }
     }
 
@@ -60,26 +60,12 @@ impl<'a, T: FieldElement> CircuitData<T> {
             .unwrap_or_else(|| panic!("{name} column not found"))
     }
 
-    pub fn len(&self) -> usize {
-        self.fixed.get(0).unwrap().1.len()
-    }
-
-    pub fn insert_constant<IT: IntoIterator<Item = T>>(
-        &mut self,
-        name: &'a str,
-        values: IT,
-    ) -> Column {
-        let values = values.into_iter().collect::<Vec<_>>();
-
-        if !self.fixed.is_empty() {
-            assert_eq!(values.len(), self.len());
-        }
-
-        self.fixed.push((name.to_string(), values));
+    pub fn insert_constant(&mut self, name: &'a str) -> Column {
         let column = Column {
             kind: ColumnKind::Fixed,
-            index: self.fixed.len() - 1,
+            index: self.fixed_id_counter,
         };
+        self.fixed_id_counter += 1;
         self.columns.insert(name.to_string(), column);
         column
     }

diff --git a/halo2/src/mock_prover.rs b/halo2/src/mock_prover.rs
@@ -26,7 +26,8 @@ pub fn mock_prove<T: FieldElement>(
     let expanded_row_count_log = circuit_row_count_log + 1;
 
     log::debug!("{}", PlafDisplayBaseTOML(&circuit.plaf));
-    println!("Fixed len: {}", circuit.plaf.fixed[0].len());
+    println!("X Fixed len: {}", circuit.plaf.fixed.len());
+    println!("Fixed[0] len: {}", circuit.plaf.fixed[0].len());
 
     let mock_prover = MockProver::<Fr>::run(expanded_row_count_log, &circuit, publics).unwrap();
     mock_prover.assert_satisfied();