Skip to content

Commit

Permalink
Fix number of rows (#3)
Browse files Browse the repository at this point in the history
* Account for empty regions

* Bump rust toolchain

Unfortunately the wasm backend makes a breaking change. We need newer version to support bumpalo
  • Loading branch information
iquerejeta authored Apr 1, 2024
1 parent 862fb67 commit 66ae3ec
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 15 deletions.
36 changes: 24 additions & 12 deletions halo2_proofs/src/dev/cost_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub struct CostOptions {

/// Minimum value of k needed to account for the circuit
pub min_k: usize,

/// Expanded rows count, which does not account for compression (where
/// multiple regions can use the same rows).
pub expanded_rows_count: usize,
Expand Down Expand Up @@ -139,7 +139,7 @@ impl Shuffle {

/// Information about how a gate (see plonk::circuit::Gate) is used in the
/// circuit.
#[derive(Clone, Debug, Deserialize, Serialize)]
#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct GateUsage {
/// The name of the gate.
pub name: String,
Expand Down Expand Up @@ -353,16 +353,29 @@ pub fn from_circuit_to_cost_model_options<F: Ord + Field + FromUniformBytes<64>,
let mut expanded_rows_count = 0;
let mut compressed_rows_count = 0;
for region in prover.regions {
if region.name.contains("table") { // Account for lookup tables separately
let (start, end) = region.rows.expect("region.rows should be known by now");
min_k = std::cmp::max(min_k, end - start);
} else {
let (start, end) = region.rows.expect("region.rows should be known by now");
expanded_rows_count = expanded_rows_count + ((end - start) + 1);
compressed_rows_count = std::cmp::max(compressed_rows_count, end + 1);
}
// If `region.rows == None`, then that region has no rows (e.g. just copy constraints)
if let Some((start, end)) = region.rows {
// Note that `end` is the index of the last row, so when
// counting rows, this last row needs to be counted via `end +
// 1`.

// A region is a _table region_ if all of its columns are `Fixed`
// columns (see that `plonk::circuit::TableColumn` is a wrapper
// around `Column<Fixed>`). All of a table region's rows are
// counted towards `table_rows_count.`
if region
.columns
.iter()
.all(|c| *c.column_type() == crate::plonk::Any::Fixed)
{
min_k = std::cmp::max(min_k, (end + 1) - start);
} else {
expanded_rows_count += (end + 1) - start;
compressed_rows_count = std::cmp::max(compressed_rows_count, end + 1);
}

min_k = std::cmp::max(min_k, compressed_rows_count);
min_k = std::cmp::max(min_k, compressed_rows_count);
}
}
(expanded_rows_count, compressed_rows_count, min_k)
};
Expand Down Expand Up @@ -390,7 +403,6 @@ pub fn from_circuit_to_cost_model_options<F: Ord + Field + FromUniformBytes<64>,
})
.collect();


CostOptions {
advice,
instance,
Expand Down
4 changes: 2 additions & 2 deletions halo2_proofs/tests/plonk_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ fn plonk_api() {
//panic!("{:#?}", pk.get_vk().pinned());
assert_eq!(
format!("{:#?}", pk.get_vk().pinned()),
r#####"PinnedVerificationKey {
r#"PinnedVerificationKey {
base_modulus: "0x40000000000000000000000000000000224698fc0994a8dd8c46eb2100000001",
scalar_modulus: "0x40000000000000000000000000000000224698fc094cf91b992d30ed00000001",
domain: PinnedEvaluationDomain {
Expand Down Expand Up @@ -1015,7 +1015,7 @@ fn plonk_api() {
(0x3d907e0591343bd285c2c846f3e871a6ac70d80ec29e9500b8cb57f544e60202, 0x1034e48df35830244cabea076be8a16d67d7896e27c6ac22b285d017105da9c3),
],
},
}"#####
}"#
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion rust-toolchain
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.67.0
1.73.0

0 comments on commit 66ae3ec

Please sign in to comment.