-
Notifications
You must be signed in to change notification settings - Fork 39
Conversation
|
||
def enable(self: Lookup, enable_annotation: str, enable_expr: Expr): | ||
enable = ASTConstraint(enable_annotation, enable_expr) | ||
if self.enable is None: | ||
enabler = ASTConstraint(enable_annotation, enable_expr) | ||
if self.enabler is None: | ||
for constraint, _ in self.exprs: | ||
constraint = self.multiply_constraints(enable, constraint) | ||
self.enable = enable | ||
constraint = self.multiply_constraints(enabler, constraint) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Called enabler
here so that it doesn't clash with the function name.
2305e64
to
38b0a5e
Compare
src/frontend/python/chiquito/dsl.py
Outdated
self.tables.add(table) | ||
if self.super_circuit is None: | ||
raise SyntaxError( | ||
"Circuit: new_table() is only available for Circuit with initiated super_circuit field." | ||
) | ||
self.super_circuit.tables[table.uuid] = table |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that here I opted to store the tables under supercircuit and then circuit/steptype both have reference to the supercircuit, so they can reference the tables as well.
src/frontend/python/mod.rs
Outdated
pub fn chiquito_super_circuit_halo2_mock_prover(rust_ids: Vec<UUID>) { | ||
let mut super_circuit_ctx = SuperCircuitContext::<Fr, ()>::default(); | ||
|
||
// super_circuit def | ||
let config = config(SingleRowCellManager {}, SimpleStepSelectorBuilder {}); | ||
for rust_id in rust_ids.clone() { | ||
let circuit_map_store = rust_id_to_halo2(rust_id); | ||
let (circuit, _, _, _) = circuit_map_store; | ||
let assignment = super_circuit_ctx.sub_circuit_with_ast(config.clone(), circuit); | ||
add_assignment_generator_to_rust_id(assignment, rust_id); | ||
} | ||
|
||
let super_circuit = super_circuit_ctx.compile(); | ||
let compiled = chiquitoSuperCircuit2Halo2(&super_circuit); | ||
|
||
let mut mapping_ctx = MappingContext::default(); | ||
for rust_id in rust_ids { | ||
let circuit_map_store = rust_id_to_halo2(rust_id); | ||
let (_, _, assignment_generator, witness) = circuit_map_store; | ||
if let Some(witness) = witness { | ||
mapping_ctx.map_with_witness(&assignment_generator.unwrap(), witness); | ||
} | ||
} | ||
|
||
let super_assignments = mapping_ctx.get_super_assignments(); | ||
|
||
let circuit = ChiquitoHalo2SuperCircuit::new(compiled, super_assignments); | ||
|
||
let prover = MockProver::<Fr>::run(10, &circuit, circuit.instance()).unwrap(); | ||
|
||
let result = prover.verify_par(); | ||
|
||
println!("result = {:#?}", result); | ||
|
||
if let Err(failures) = &result { | ||
for failure in failures.iter() { | ||
println!("{}", failure); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chiquito_super_circuit_halo2_mock_prover
def sub_circuit(self: SuperCircuit, sub_circuit: Circuit) -> Circuit: | ||
assert self.mode == SuperCircuitMode.SETUP | ||
if sub_circuit.rust_id != 0: | ||
raise ValueError( | ||
"SuperCircuit: sub_circuit() cannot be called twice on the same circuit." | ||
) | ||
ast_json: str = sub_circuit.get_ast_json() | ||
sub_circuit.rust_id: int = rust_chiquito.ast_to_halo2(ast_json) | ||
self.ast.sub_circuits[sub_circuit.rust_id] = sub_circuit.ast | ||
return sub_circuit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sub_circuit
Python function.
I'd comment that historically we only serialize a That's actually what I did, to serialize a mapping of
We first call Then in Rust, the Just to clarify, I created a
This is how all corresponding items are grouped under the same Again, for your convenience, I'm marking the |
src/frontend/python/chiquito/dsl.py
Outdated
# called at the outermost level | ||
# generates TraceWitness mapping | ||
def gen_witness(self: SuperCircuit, args: Any) -> Dict[int, TraceWitness]: | ||
self.mode = SuperCircuitMode.Mapping | ||
self.mapping(args) | ||
self.mode = SuperCircuitMode.NoMode | ||
witnesses: Dict[int, TraceWitness] = self.ast.witnesses | ||
del ( | ||
self.ast.witnesses | ||
) # so that we can generate different witness mapping in the next gen_witness() call | ||
return witnesses |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gen_witness
Python function for SuperCircuit
src/frontend/python/chiquito/dsl.py
Outdated
def halo2_mock_prover(self: SuperCircuit, witnesses: Dict[int, TraceWitness]): | ||
for rust_id, witness in witnesses.items(): | ||
witness_json: str = witness.get_witness_json() | ||
if rust_id not in self.ast.sub_circuits: | ||
raise ValueError( | ||
f"SuperCircuit.halo2_mock_prover(): TraceWitness with rust_id {rust_id} not found in sub_circuits." | ||
) | ||
rust_chiquito.add_witness_to_rust_id(witness_json, rust_id) | ||
rust_chiquito.super_circuit_halo2_mock_prover( | ||
list(self.ast.sub_circuits.keys()) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
halo2_mock_prover
Python function for SuperCircuit
Ready for review again. Mostly responding to comments. I also merged latest changes. @leolara |
This PR now contains both fixed gen refactoring from #117 and super circuit related changes. @leolara Ready for final review. Changes:
|
examples/mimc7.py
Outdated
self.transition(eq(self.circuit.row + 1, self.circuit.row.next())) | ||
|
||
self.add_lookup( | ||
self.circuit.imports.apply(self.circuit.row).apply(self.circuit.c) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qwang98 I think it makes sense to call it self.circuit.constants_table
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. I replaced the imports parameter for Circuit
as **kwargs
, so that the user can pass in arbitrarily named parameters for imports.
self.add(self.mimc7_last_step, x_value, k_value, c_value, row_value) | ||
|
||
|
||
class Mimc7FirstStep(StepType): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qwang98 I think it is more clear to put the steps before the circuit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
src/frontend/dsl/mod.rs
Outdated
@@ -172,6 +162,27 @@ impl<F, TraceArgs> CircuitContext<F, TraceArgs> { | |||
} | |||
} | |||
|
|||
impl<F: Field + Hash, TraceArgs> CircuitContext<F, TraceArgs> { | |||
/// Sets the fixed generation function for the circuit. The fixed generation function is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qwang98 this does not "set", but "executes" and sets the assignations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
src/frontend/python/mod.rs
Outdated
Circuit<Fr, ()>, | ||
ChiquitoHalo2<Fr>, | ||
Option<AssignmentGenerator<Fr, ()>>, | ||
// Option<TraceWitness<Fr>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qwang98 remove commented out line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
src/frontend/python/mod.rs
Outdated
circuit_map_store.2 = Some(assignment_generator); | ||
}); | ||
|
||
println!("Added AssignmentGenerator to rust_id: {:?}", rust_id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qwang98 remove println
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few comments, but it is approved
Will be PR'ed to main.
@leolara Ready for review and the MiMC7 example is fully functional.
My technical approach
halo2_mock_prover
function takes vector of uuids and compiles super circuit before passing it to the mock prover.