PIL LogUp implementation: Generate multiplicity column #2042
Conversation
| @@ -174,7 +174,7 @@ impl<T: FieldElement> IndexedColumns<T> { | |||
| } | |||
| } | |||
|
|
|||
| const MULTIPLICITY_LOOKUP_COLUMN: &str = "m_logup_multiplicity"; | |||
| const MULTIPLICITY_LOOKUP_COLUMN: &str = "multiplicities"; | |||
There was a problem hiding this comment.
Planning to remove this ASAP (prototype in #1958) and get this from the phantom lookups.
| let column = columns.remove(&name).unwrap(); | ||
| let column = columns | ||
| .remove(&name) | ||
| .unwrap_or_else(|| panic!("No machine generated witness for column: {name}")); |
There was a problem hiding this comment.
I don't understand this, in which situations can this fail?
std/protocols/lookup.asm
Outdated
| /// - lookup_constraint: The lookup constraint | ||
| /// - multiplicities: A multiplicities column which shows how many times each row of the RHS value appears in the LHS | ||
| let lookup: Constr, expr -> () = constr |lookup_constraint, multiplicities| { | ||
| /// - lookup_constraint: The lookup constraint |
There was a problem hiding this comment.
Quite redundant, I would just remove it.
std/protocols/lookup.asm
Outdated
| @@ -75,6 +74,7 @@ let lookup: Constr, expr -> () = constr |lookup_constraint, multiplicities| { | |||
|
|
|||
| let lhs_denom = sub_ext(beta, fingerprint(lhs, alpha)); | |||
| let rhs_denom = sub_ext(beta, fingerprint(rhs, alpha)); | |||
| let multiplicities = std::prover::new_witness_col_at_stage("multiplicities", 0); | |||
There was a problem hiding this comment.
Why specify the stage explicitly?
There was a problem hiding this comment.
If you use let multiplicities, it will be create at the "current stage".
There was a problem hiding this comment.
Right, if we use capture_stage, then we might already be at a higher stage, but I think there should still be a better way around that than just setting the stage to zero explicitly. I think I would just use let multiplicites now until we reach that problem.
No need to have the user create the multiplicity column anymore.