Remove tracking of range constraint multiplicities (#2356)

georgwiese · web-flow · commit fd576d51eb83 · 2025-01-20T10:43:13.000Z
This is not necessary anymore since #2319: We previously had a multiplicity witgen pass only for range constraints via lookups. Now we have it for all phantom lookups, which includes range constraints via lookups.
diff --git a/executor/src/witgen/global_constraints.rs b/executor/src/witgen/global_constraints.rs
@@ -18,7 +18,6 @@ use crate::Identity;
 use super::affine_expression::AlgebraicVariable;
 use super::evaluators::partial_expression_evaluator::PartialExpressionEvaluator;
 use super::evaluators::symbolic_evaluator::SymbolicEvaluator;
-use super::machines::Connection;
 use super::range_constraints::RangeConstraint;
 use super::util::try_to_simple_poly;
 use super::{Constraint, FixedData};
@@ -96,20 +95,10 @@ where
     }
 }
 
-#[derive(Clone, Debug, PartialEq, Eq)]
-/// The RHS of a range constraint of the form `[ w ] in [ RANGE ]`, including the multiplicity column.
-pub struct PhantomRangeConstraintTarget {
-    pub column: PolyID,
-    pub multiplicity_column: PolyID,
-}
-
 #[derive(Clone)]
 pub struct GlobalConstraints<T: FieldElement> {
     pub witness_constraints: WitnessColumnMap<Option<RangeConstraint<T>>>,
     pub fixed_constraints: FixedColumnMap<Option<RangeConstraint<T>>>,
-    /// For range constraints which are enforced via phantom lookups, this maps the
-    /// constrained column to the target and multiplicity column.
-    pub phantom_range_constraints: BTreeMap<PolyID, PhantomRangeConstraintTarget>,
 }
 
 impl<T: FieldElement> RangeConstraintSet<&AlgebraicReference, T> for GlobalConstraints<T> {
@@ -153,12 +142,10 @@ pub fn set_global_constraints<'a, T: FieldElement>(
 
     let mut retained_identities = vec![];
     let mut removed_identities = vec![];
-    let mut range_constraint_multiplicities = BTreeMap::new();
     for identity in identities.into_iter() {
         let remove = propagate_constraints(
             &fixed_data.intermediate_definitions,
             &mut known_constraints,
-            &mut range_constraint_multiplicities,
             identity,
             &full_span,
         );
@@ -183,17 +170,6 @@ pub fn set_global_constraints<'a, T: FieldElement>(
         log::debug!("  {id}");
     }
 
-    if !range_constraint_multiplicities.is_empty() {
-        log::debug!("Recorded the following range constraint multiplicity columns:");
-    }
-    for (poly_id, target) in &range_constraint_multiplicities {
-        log::debug!(
-            "  {} -> {}",
-            fixed_data.column_name(poly_id),
-            fixed_data.column_name(&target.multiplicity_column)
-        );
-    }
-
     let mut witness_constraints: WitnessColumnMap<Option<RangeConstraint<T>>> =
         fixed_data.witness_map_with(None);
     for (poly_id, con) in known_constraints {
@@ -211,7 +187,6 @@ pub fn set_global_constraints<'a, T: FieldElement>(
     let global_constraints = GlobalConstraints {
         witness_constraints,
         fixed_constraints,
-        phantom_range_constraints: range_constraint_multiplicities,
     };
 
     (
@@ -260,7 +235,6 @@ fn add_constraint<T: FieldElement>(
 fn propagate_constraints<T: FieldElement>(
     intermediate_definitions: &BTreeMap<AlgebraicReferenceThin, AlgebraicExpression<T>>,
     known_constraints: &mut BTreeMap<PolyID, RangeConstraint<T>>,
-    range_constraint_multiplicities: &mut BTreeMap<PolyID, PhantomRangeConstraintTarget>,
     identity: &Identity<T>,
     full_span: &BTreeSet<PolyID>,
 ) -> bool {
@@ -303,21 +277,14 @@ fn propagate_constraints<T: FieldElement>(
             // In that case, we can remove the lookup, because its only function is to enforce
             // the range constraint.
             if right.expressions.len() == 1 {
-                if let (Some(left_ref), Some(right_ref)) = (
+                if let (Some(_), Some(right_ref)) = (
+                    // The range constraint has been transferred from right to left
+                    // by the above code iff. both expressions can be converted to
+                    // simple polynomials.
                     try_to_simple_poly(&left.expressions[0]),
                     try_to_simple_poly(&right.expressions[0]),
                 ) {
                     if full_span.contains(&right_ref.poly_id) {
-                        let connection = Connection::try_from(identity).unwrap();
-                        if let Some(multiplicity) = connection.multiplicity_column {
-                            let target = PhantomRangeConstraintTarget {
-                                column: right_ref.poly_id,
-                                multiplicity_column: multiplicity,
-                            };
-                            assert!(range_constraint_multiplicities
-                                .insert(left_ref.poly_id, target)
-                                .is_none());
-                        }
                         return true;
                     }
                 }
@@ -557,12 +524,10 @@ namespace Global(2**20);
             .into_iter()
             .collect()
         );
-        let mut range_constraint_multiplicities = BTreeMap::new();
         for identity in &analyzed.identities {
             propagate_constraints(
                 &BTreeMap::new(),
                 &mut known_constraints,
-                &mut range_constraint_multiplicities,
                 identity,
                 &full_span,
             );
@@ -649,12 +614,10 @@ namespace Global(2**20);
             .into_iter()
             .collect()
         );
-        let mut range_constraint_multiplicities = BTreeMap::new();
         for identity in &analyzed.identities {
             propagate_constraints(
                 &BTreeMap::new(),
                 &mut known_constraints,
-                &mut range_constraint_multiplicities,
                 identity,
                 &full_span,
             );
@@ -680,33 +643,6 @@ namespace Global(2**20);
             .into_iter()
             .collect::<BTreeMap<_, _>>()
         );
-        assert_eq!(
-            range_constraint_multiplicities,
-            vec![
-                (
-                    // Global.B
-                    witness_poly_id(3),
-                    PhantomRangeConstraintTarget {
-                        // Global.BYTE
-                        column: constant_poly_id(0),
-                        // Global.byte_multiplicities
-                        multiplicity_column: witness_poly_id(0)
-                    }
-                ),
-                (
-                    // Global.D
-                    witness_poly_id(5),
-                    PhantomRangeConstraintTarget {
-                        // Global.BYTE
-                        column: constant_poly_id(0),
-                        // Global.byte_multiplicities
-                        multiplicity_column: witness_poly_id(0)
-                    }
-                ),
-            ]
-            .into_iter()
-            .collect::<BTreeMap<_, _>>()
-        )
     }
 
     #[test]
@@ -726,12 +662,10 @@ namespace Global(1024);
         let mut known_constraints = vec![(constant_poly_id(0), RangeConstraint::from_max_bit(7))]
             .into_iter()
             .collect();
-        let mut range_constraint_multiplicities = BTreeMap::new();
         assert_eq!(analyzed.identities.len(), 1);
         let removed = propagate_constraints(
             &BTreeMap::new(),
             &mut known_constraints,
-            &mut range_constraint_multiplicities,
             analyzed.identities.first().unwrap(),
             &Default::default(),
         );
diff --git a/executor/src/witgen/machines/machine_extractor.rs b/executor/src/witgen/machines/machine_extractor.rs
@@ -79,17 +79,7 @@ impl<'a, T: FieldElement> MachineExtractor<'a, T> {
         let mut machines: Vec<KnownMachine<T>> = vec![];
 
         let mut publics = PublicsTracker::default();
-        let range_constraint_multiplicities = self
-            .fixed
-            .global_range_constraints
-            .phantom_range_constraints
-            .values()
-            .map(|prc| prc.multiplicity_column)
-            .collect::<HashSet<_>>();
-        let mut remaining_witnesses = current_stage_witnesses
-            .difference(&range_constraint_multiplicities)
-            .cloned()
-            .collect::<HashSet<_>>();
+        let mut remaining_witnesses = current_stage_witnesses.clone();
         let mut base_identities = identities.clone();
         let mut extracted_prover_functions = HashSet::new();
         let mut id_counter = 0;
@@ -222,24 +212,40 @@ impl<'a, T: FieldElement> MachineExtractor<'a, T> {
             .filter_map(|(i, &pf)| (!extracted_prover_functions.contains(&i)).then_some(pf))
             .collect::<Vec<_>>();
 
+        // In the remaining witness, we might still have some multiplicity columns
+        // of fixed lookups, because they are not referenced by any "normal"
+        // first-stage identities. As the main machine should not be on the
+        // receiving end of a lookup, we remove any multiplicity columns here.
+        // Note that we don't use the passed identities, because we want to
+        // include removed range constraints.
+        let multiplicity_columns = self
+            .fixed
+            .analyzed
+            .identities
+            .iter()
+            .filter_map(|identity| Connection::try_from(identity).ok()?.multiplicity_column)
+            .collect::<HashSet<_>>();
+        let main_witnesses = remaining_witnesses
+            .difference(&multiplicity_columns)
+            .cloned()
+            .collect::<HashSet<_>>();
+
         log::trace!(
             "\nThe base machine contains the following witnesses:\n{}\n identities:\n{}\n and prover functions:\n{}",
-            remaining_witnesses
+            main_witnesses
                 .iter()
                 .map(|s| self.fixed.column_name(s))
                 .sorted()
                 .format(", "),
-            base_identities
-                .iter()
-                .format("\n"),
+            base_identities.iter().format("\n"),
             base_prover_functions.iter().format("\n")
         );
 
         let base_parts = MachineParts::new(
             self.fixed,
             Default::default(),
             base_identities,
-            remaining_witnesses,
+            main_witnesses,
             base_prover_functions,
         );
 
diff --git a/executor/src/witgen/mod.rs b/executor/src/witgen/mod.rs
@@ -379,7 +379,6 @@ impl<'a, T: FieldElement> FixedData<'a, T> {
         let global_range_constraints = GlobalConstraints {
             witness_constraints: WitnessColumnMap::new(None, witness_cols.len()),
             fixed_constraints: FixedColumnMap::new(None, fixed_cols.len()),
-            phantom_range_constraints: BTreeMap::new(),
         };
 
         FixedData {
