Bus using stwo logup #2505
Bus using stwo logup #2505
Conversation
| // add selector columns for the bus accumulator | ||
| let mut col = Col::<SimdBackend, BaseField>::zeros(1 << log_size); | ||
| for index in 0..(1 << log_size) { | ||
| col.set(index, BaseField::one()); | ||
| } | ||
| col.set(0, BaseField::zero()); | ||
| let selector = CircleEvaluation::< | ||
| SimdBackend, | ||
| BaseField, | ||
| BitReversedOrder, | ||
| >::new( | ||
| *domain_map.get(&(log_size as usize)).unwrap(), | ||
| col, | ||
| ); |
There was a problem hiding this comment.
Is this a column with 1 everywhere except 0 in the first row?
Another note is that currently we haven't exposed the bus accumulator in other backends, because our publics infrastructure requires exposing a specific row of a witness column, but we can't know which row in the accumulator column to expose. I'm actually working on this right now so eventually we can expose bus accumulator.
| let mut bus_id_to_interaction_map = BTreeMap::new(); | ||
| let mut index = 2; | ||
|
|
||
| self.split.iter().for_each(|(machine_name, pil)| { | ||
| for identity in &pil.identities { | ||
| if let Identity::PhantomBusInteraction(id) = identity { | ||
| // println!( | ||
| // "bus is found in machine: {}, with bus id {}", | ||
| // machine_name, id | ||
| // ); | ||
| index += 1; | ||
| bus_id_to_interaction_map | ||
| .insert(index, (id.bus_id.clone(), machine_name.clone(), id.id)); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
This index here is called interaction_id below and isn't used. I wonder what is it intended for and why does it start from 2?
There was a problem hiding this comment.
it is the index for merkle tree to commit witness columns. In stwo implementation now, all the constant columns are committed by merkle tree 0, stage 0 witness columns is merkle tree 1, stage 1 witness columns is merkle tree 2, see these constants are encoded here, and they are used here when building new columns in the circuit. I use one merkle tree for each bus interactions so the accumulator for each bus is easier to be indexed, but this gives extra cost. Less merkle trees is better. Now it is mainly for debugging purpose.
| let log_size_payload = machine_log_sizes.get(machine_name).unwrap().clone(); | ||
|
|
||
| let mut payload_cols = Vec::<_>::new(); | ||
| // the bus id.id can change if it is accessed from analyze or split pil |
There was a problem hiding this comment.
What do you mean here? Do you mean the unique id for identities in a machine can change?
There was a problem hiding this comment.
I noticed the identity id changed depends on if I access it from analyzed for all the machines, or I accessed it after split, namely the pil for individual machine.
| let q: PackedSecureField = | ||
| lookup_elements.combine(&payload_denom_values); |
There was a problem hiding this comment.
Is this basically doing linear combination with challenge \alpha (lookup_elements) like \sum_i {\alpha^i * payload_i}?
I think PhantomBusInteractionIdentity has a folded_expressions field, which should have done this exact same calculation. Am I understanding correctly that we need to do this calculation here manually in order to use Stwo's native LogUp?
| pub struct PowdrLogupTraceGenerator { | ||
| log_size: u32, | ||
| /// Current allocated interaction columns. | ||
| pub trace: Vec<SecureColumnByCoords<SimdBackend>>, | ||
| /// Denominator expressions (z + sum_i alpha^i * x_i) being generated for the current lookup. | ||
| denom: SecureColumn, | ||
| } |
There was a problem hiding this comment.
denom is set to the trace generator while numerator the column generator. Is it because many trace columns share the same denominator?
| let value = self.gen.trace.last().unwrap().packed_at(vec_row); | ||
| let acc_pack_array: [QM31; 16] = value | ||
| .to_array() | ||
| .iter() | ||
| .scan(last_value, |state, &x| { | ||
| *state += x; | ||
| Some(*state) | ||
| }) | ||
| .collect::<Vec<QM31>>() // Collect into a Vec<QM31> | ||
| .try_into() // Convert Vec<QM31> into [QM31; 16] | ||
| .expect("Expected exactly 16 elements"); | ||
| last_value=acc_pack_array[15]; | ||
|
|
||
| let acc_pack = PackedQM31::from_array(acc_pack_array); |
There was a problem hiding this comment.
Is this the sum of all accumulators (seems to be so from the scan)? Why we expect exactly 16 elements? Is it multiplicity_0 / folded_payload_0 + multiplicity_1 / folded_payload_1, so 4 sets of 4 columns for each extension field = 16 elements?
There was a problem hiding this comment.
I think my guess isn't correct from a further read of the code.
| let trace = self | ||
| .gen | ||
| .trace | ||
| .clone() | ||
| .iter() | ||
| .flat_map(|eval| { | ||
| eval.clone().columns.map(|col| { | ||
| let nature_order_col = col.into_cpu_vec(); | ||
| let mut bit_reversed_col =Col::<SimdBackend, BaseField>::zeros(1 << self.log_size); | ||
| for index in 0..(1 << self.log_size) { | ||
| bit_reversed_col.set( | ||
| bit_reverse_index( | ||
| coset_index_to_circle_domain_index( | ||
| index, | ||
| self.log_size, | ||
| ), | ||
| self.log_size, | ||
| ), | ||
| nature_order_col[index], | ||
| ); | ||
| } | ||
| CircleEvaluation::new(CanonicCoset::new(self.log_size).circle_domain(), bit_reversed_col) | ||
| }) | ||
| }) | ||
| .collect_vec(); |
| // build bus mask offsets | ||
| //TODO: This constant column needs to be added to setup | ||
| let selector_not_first = eval.get_preprocessed_column(PreProcessedColumnId { | ||
| id: (self.publics_values.len() | ||
| + constant_eval.len() | ||
| + self.preprocess_col_offset | ||
| + constant_shifted_eval.len()) | ||
| .to_string(), | ||
| }); | ||
|
|
||
| let bus_accumulator_eval: BTreeMap<_, _> = self | ||
| .bus_info_to_interation_map | ||
| .iter() | ||
| .filter(|(_, (_, machine_name, _))| *machine_name == self.machine_name) | ||
| .map(|(interaction_index, (bus_id, machine_name, identity_id))| { | ||
| println!("in evaluate function, machine name is {}, bus identity id is {}, interaction index is {}", machine_name, identity_id, interaction_index); | ||
| ( | ||
| identity_id, | ||
| eval.next_extension_interaction_mask(*interaction_index as usize, [-1, 0]), | ||
| ) | ||
| }) | ||
| .collect(); |
There was a problem hiding this comment.
I assume the purpose here is to add an "expose public" style constraint for bus accumulators. Again, there's no way to know which row of the bus accumulator to expose given our current "expose a row in witness trace" style API for publics. Would need to wait till publics are decoupled from trace rows implemented.
| // build bus mask offsets | ||
| //TODO: This constant column needs to be added to setup | ||
| let selector_not_first = eval.get_preprocessed_column(PreProcessedColumnId { | ||
| id: (self.publics_values.len() | ||
| + constant_eval.len() | ||
| + self.preprocess_col_offset | ||
| + constant_shifted_eval.len()) | ||
| .to_string(), | ||
| }); | ||
|
|
||
| let bus_accumulator_eval: BTreeMap<_, _> = self | ||
| .bus_info_to_interation_map | ||
| .iter() | ||
| .filter(|(_, (_, machine_name, _))| *machine_name == self.machine_name) | ||
| .map(|(interaction_index, (bus_id, machine_name, identity_id))| { | ||
| println!("in evaluate function, machine name is {}, bus identity id is {}, interaction index is {}", machine_name, identity_id, interaction_index); | ||
| ( | ||
| identity_id, | ||
| eval.next_extension_interaction_mask(*interaction_index as usize, [-1, 0]), | ||
| ) | ||
| }) | ||
| .collect(); |
There was a problem hiding this comment.
I assume the purpose here is to add an "expose public" style constraint for bus accumulators. Again, there's no way to know which row of the bus accumulator to expose given our current "expose a row in witness trace" style API for publics. Would need to wait till publics are decoupled from trace rows implemented.
| eval.add_constraint::<<E as EvalAtRow>::EF>( | ||
| <E as EvalAtRow>::EF::from(selector_not_first.clone()) | ||
| * ((bus_accumulator_eval.get(&id.id).unwrap()[1].clone() | ||
| - bus_accumulator_eval.get(&id.id).unwrap()[0].clone()) | ||
| * denominator | ||
| - multiplicity), | ||
| ); |
There was a problem hiding this comment.
This seems to be the main bus constraint. Is this the "native log up" we are talking about? I'd also note that in our current PIL function call for adding constraints (the non native log up way), we batch two bus interactions for each accumulator column as acc * selector_not_first + mult_0/denom_0 + mult_1/denom_1 = acc'. We technically can batch as many mult/denom as possible, but currently enforce a degree 3 constraint due to the limit from Plonky3 backend. I think your backend log up native implementation here with one mult/denom per accumulator column is good as a first step, but as some future optimization, we can consider batching multiple mult/denom per accumulator :) Relatedly, do we have a maximum degree for our set up of Stwo?
This PR implements the bus via stwo native logup.
stwo provides a evaluate function that accepts extension field element. for bus constraints the following relations are to be built:
in the verification side, verify the sum of the publics with the same bus_id is 0.
currently these relation information is extract from
PhantomBusInteraction, acc is built in the backend. The function I used from stwo to build acc in extension field has SIMD features, it cannot accept witness with length less than 16 (the parellized computation unit size)the
shift_small_test.asm, bus_lookup.asm (modified to size 16), bus_multi_linker.asm, bus_multi_lookup.asm, memory_small_test.asmcan be proved successfully. With current bus interface, this means the backend stwo proves bus relations built by powdr pil and the extra stwo native logup relations extracted from PhantomBusInteraction.would be good to have a compiled pil file, with PhantomBusInteraction identity but without stage 1 witness.