Skip to content

Commit

Permalink
mock prover hide duplicate errors (#354)
Browse files Browse the repository at this point in the history
Currently, since we are adding a duplicate instance at the end, this
causes duplicate errors and sometimes it can be a lot of errors. This PR
attempts to cover the current case where if two consecutive errors are
identical then hide the duplicate.

```
======================================================

AssertZeroError ....

Error: 2 constraints not satisfied (1 duplicates hidden)
======================================================
```
  • Loading branch information
zemse authored Oct 14, 2024
1 parent 353858e commit c0fd609
Showing 1 changed file with 101 additions and 7 deletions.
108 changes: 101 additions & 7 deletions ceno_zkvm/src/scheme/mock_prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ pub const MOCK_PC_LUI: ByteAddr = ByteAddr(CENO_PLATFORM.pc_start() + 88);
pub const MOCK_PC_AUIPC: ByteAddr = ByteAddr(CENO_PLATFORM.pc_start() + 92);

#[allow(clippy::enum_variant_names)]
#[derive(Debug, PartialEq, Clone)]
#[derive(Debug, Clone)]
pub(crate) enum MockProverError<E: ExtensionField> {
AssertZeroError {
expression: Expression<E>,
Expand All @@ -143,6 +143,75 @@ pub(crate) enum MockProverError<E: ExtensionField> {
// w_expressions
}

impl<E: ExtensionField> PartialEq for MockProverError<E> {
// Compare errors based on the content, ignoring the inst_id
fn eq(&self, other: &Self) -> bool {
match (self, other) {
(
MockProverError::AssertZeroError {
expression: left_expression,
evaluated: left_evaluated,
name: left_name,
..
},
MockProverError::AssertZeroError {
expression: right_expression,
evaluated: right_evaluated,
name: right_name,
..
},
) => {
left_expression == right_expression
&& left_evaluated == right_evaluated
&& left_name == right_name
}
(
MockProverError::AssertEqualError {
left_expression: left_left_expression,
right_expression: left_right_expression,
left: left_left,
right: left_right,
name: left_name,
..
},
MockProverError::AssertEqualError {
left_expression: right_left_expression,
right_expression: right_right_expression,
left: right_left,
right: right_right,
name: right_name,
..
},
) => {
left_left_expression == right_left_expression
&& left_right_expression == right_right_expression
&& left_left == right_left
&& left_right == right_right
&& left_name == right_name
}
(
MockProverError::LookupError {
expression: left_expression,
evaluated: left_evaluated,
name: left_name,
..
},
MockProverError::LookupError {
expression: right_expression,
evaluated: right_evaluated,
name: right_name,
..
},
) => {
left_expression == right_expression
&& left_evaluated == right_evaluated
&& left_name == right_name
}
_ => false,
}
}
}

impl<E: ExtensionField> MockProverError<E> {
pub fn print(&self, wits_in: &[ArcMultilinearExtension<E>], wits_in_name: &[String]) {
let mut wtns = vec![];
Expand Down Expand Up @@ -203,6 +272,14 @@ impl<E: ExtensionField> MockProverError<E> {
}
}
}

fn inst_id(&self) -> usize {
match self {
Self::AssertZeroError { inst_id, .. }
| Self::AssertEqualError { inst_id, .. }
| Self::LookupError { inst_id, .. } => *inst_id,
}
}
}

pub(crate) struct MockProver<E: ExtensionField> {
Expand Down Expand Up @@ -419,7 +496,7 @@ impl<'a, E: ExtensionField + Hash> MockProver<E> {
}
} else {
// contains require_zero
let expr_evaluated = wit_infer_by_expr(&[], wits_in, pi, &challenge, &expr);
let expr_evaluated = wit_infer_by_expr(&[], wits_in, pi, &challenge, expr);
let expr_evaluated = expr_evaluated
.get_ext_field_vec_optn()
.map(|v| v.to_vec())
Expand Down Expand Up @@ -488,7 +565,6 @@ impl<'a, E: ExtensionField + Hash> MockProver<E> {
Ok(_) => {}
Err(errors) => {
println!("======================================================");
println!("Error: {} constraints not satisfied", errors.len());

println!(
r"Hints:
Expand All @@ -499,8 +575,26 @@ impl<'a, E: ExtensionField + Hash> MockProver<E> {
"
);

for error in errors {
error.print(wits_in, &cb.cs.witin_namespace_map);
// Print errors and skip consecutive duplicates errors if they are equal.
let mut duplicates = 0;
let mut prev_err = None;
for error in &errors {
if prev_err.is_some() && prev_err.unwrap() == error {
duplicates += 1;
} else {
error.print(wits_in, &cb.cs.witin_namespace_map);
}
prev_err = Some(error);
}

if duplicates > 0 {
println!(
"Error: {} constraints not satisfied ({} duplicates hidden)",
errors.len(),
duplicates
);
} else {
println!("Error: {} constraints not satisfied", errors.len());
}
println!("======================================================");
panic!("Constraints not satisfied");
Expand Down Expand Up @@ -638,8 +732,6 @@ mod tests {
Box::new(Expression::Challenge(
1,
1,
// TODO this still uses default challenge in ConstraintSystem, but challengeId
// helps to evaluate the expression correctly. Shoudl challenge be just challengeId?
GoldilocksExt2::ONE,
GoldilocksExt2::ZERO,
)),
Expand All @@ -659,6 +751,8 @@ mod tests {
inst_id: 0,
}]
);
// because inst_id is not checked in our PartialEq impl
assert_eq!(err[0].inst_id(), 0);
}

#[allow(dead_code)]
Expand Down

0 comments on commit c0fd609

Please sign in to comment.