Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: check thread-safe-region feature(post-split) #296

Merged
merged 10 commits into from
Mar 28, 2024
Merged

fix: check thread-safe-region feature(post-split) #296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b1b9be5
feat: remove the "thread-safe-region" feature from "halo2_frontend"
duguorong009 Mar 11, 2024
f3f6c27
fix: re-enable the "thread-safe-region" for "backend::plonk::keygen::…
duguorong009 Mar 11, 2024
e89dd24
fix: re-enable the "thread-safe-region" feature in "halo2_frontend"
duguorong009 Mar 11, 2024
670d620
chore: fmt + clippy
duguorong009 Mar 11, 2024
3a8972a
Merge branch 'main' into gr@post-split-check-thread-safe-region
duguorong009 Mar 22, 2024
6078446
Merge branch 'main' into gr@post-split-check-thread-safe-region
duguorong009 Mar 22, 2024
3f60436
feat: remove "thread-safe-region" feature & its code in backend
duguorong009 Mar 26, 2024
f987236
fix: add the sorting of "assembly.permutation.copies"
duguorong009 Mar 26, 2024
c039c29
fix: apply the suggestions
duguorong009 Mar 27, 2024
7ad1f43
fix: add the "thread-safe-region" feature before sorting copies
duguorong009 Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
212 changes: 0 additions & 212 deletions halo2_backend/src/plonk/permutation/keygen.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,6 @@ use crate::{
use halo2_middleware::circuit::ColumnMid;
use halo2_middleware::permutation::{ArgumentMid, AssemblyMid};

// NOTE: Temporarily disabled thread-safe-region feature. Regions are a frontend concept, so the
// thread-safe support for them should be only in the frontend package.
// TODO: Bring the thread-safe region feature back
// https://github.com/privacy-scaling-explorations/halo2/issues/258

// #[cfg(feature = "thread-safe-region")]
// use crate::multicore::{IndexedParallelIterator, IntoParallelIterator, ParallelIterator};

/*
#[cfg(feature = "thread-safe-region")]
use std::collections::{BTreeSet, HashMap};
*/

// #[cfg(not(feature = "thread-safe-region"))]
/// Struct that accumulates all the necessary data in order to construct the permutation argument.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Assembly {
Expand All @@ -40,7 +26,6 @@ pub struct Assembly {
sizes: Vec<Vec<usize>>,
}

// #[cfg(not(feature = "thread-safe-region"))]
impl Assembly {
pub(crate) fn new_from_assembly_mid(
n: usize,
Expand Down Expand Up @@ -150,203 +135,6 @@ impl Assembly {
}
}

/*
#[cfg(feature = "thread-safe-region")]
/// Struct that accumulates all the necessary data in order to construct the permutation argument.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Assembly {
/// Columns that participate on the copy permutation argument.
columns: Vec<ColumnMid>,
/// Mapping of the actual copies done.
cycles: Vec<Vec<(usize, usize)>>,
/// Mapping of the actual copies done.
ordered_cycles: Vec<BTreeSet<(usize, usize)>>,
/// Mapping of the actual copies done.
aux: HashMap<(usize, usize), usize>,
/// total length of a column
col_len: usize,
/// number of columns
num_cols: usize,
}

#[cfg(feature = "thread-safe-region")]
impl Assembly {
pub(crate) fn new_from_assembly_mid(
n: usize,
p: &ArgumentV2,
a: &AssemblyMid,
) -> Result<Self, Error> {
let mut assembly = Self::new(n, &p.clone().into());
for copy in &a.copies {
assembly.copy(copy.0.column, copy.0.row, copy.1.column, copy.1.row)?;
}
Ok(assembly)
}

pub(crate) fn new(n: usize, p: &Argument) -> Self {
// Initialize the copy vector to keep track of copy constraints in all
// the permutation arguments.
let mut columns = vec![];
for i in 0..p.columns.len() {
// Computes [(i, 0), (i, 1), ..., (i, n - 1)]
columns.push((0..n).map(|j| (i, j)).collect());
}

Assembly {
columns: p.columns.clone().into_iter().map(|c| c.into()).collect(),
mapping: columns.clone(),
aux: columns,
sizes: vec![vec![1usize; n]; p.columns.len()],
}
}

pub(crate) fn copy(
&mut self,
left_column: ColumnMid,
left_row: usize,
right_column: ColumnMid,
right_row: usize,
) -> Result<(), Error> {
let left_column = self
.columns
.iter()
.position(|c| c == &left_column)
.ok_or(Error::ColumnNotInPermutation(left_column))?;
let right_column = self
.columns
.iter()
.position(|c| c == &right_column)
.ok_or(Error::ColumnNotInPermutation(right_column))?;

// Check bounds
if left_row >= self.col_len || right_row >= self.col_len {
return Err(Error::BoundsFailure);
}

let left_cycle = self.aux.get(&(left_column, left_row));
let right_cycle = self.aux.get(&(right_column, right_row));

// extract cycle elements
let right_cycle_elems = match right_cycle {
Some(i) => {
let entry = self.cycles[*i].clone();
self.cycles[*i] = vec![];
entry
}
None => [(right_column, right_row)].into(),
};

assert!(right_cycle_elems.contains(&(right_column, right_row)));

// merge cycles
let cycle_idx = match left_cycle {
Some(i) => {
let entry = &mut self.cycles[*i];
entry.extend(right_cycle_elems.clone());
*i
}
// if they were singletons -- create a new cycle entry
None => {
let mut set: Vec<(usize, usize)> = right_cycle_elems.clone();
set.push((left_column, left_row));
self.cycles.push(set);
let cycle_idx = self.cycles.len() - 1;
self.aux.insert((left_column, left_row), cycle_idx);
cycle_idx
}
};

let index_updates = vec![cycle_idx; right_cycle_elems.len()].into_iter();
let updates = right_cycle_elems.into_iter().zip(index_updates);

self.aux.extend(updates);

Ok(())
}

/// Builds the ordered mapping of the cycles.
/// This will only get executed once.
pub fn build_ordered_mapping(&mut self) {
use crate::multicore::IntoParallelRefMutIterator;

// will only get called once
if self.ordered_cycles.is_empty() && !self.cycles.is_empty() {
self.ordered_cycles = self
.cycles
.par_iter_mut()
.map(|col| {
let mut set = BTreeSet::new();
set.extend(col.clone());
// free up memory
*col = vec![];
set
})
.collect();
}
}

fn mapping_at_idx(&self, col: usize, row: usize) -> (usize, usize) {
assert!(
!self.ordered_cycles.is_empty() || self.cycles.is_empty(),
"cycles have not been ordered"
);

if let Some(cycle_idx) = self.aux.get(&(col, row)) {
let cycle = &self.ordered_cycles[*cycle_idx];
let mut cycle_iter = cycle.range((
std::ops::Bound::Excluded((col, row)),
std::ops::Bound::Unbounded,
));
// point to the next node in the cycle
match cycle_iter.next() {
Some((i, j)) => (*i, *j),
// wrap back around to the first element which SHOULD exist
None => *(cycle.iter().next().unwrap()),
}
// is a singleton
} else {
(col, row)
}
}

pub(crate) fn build_vk<'params, C: CurveAffine, P: Params<'params, C>>(
&mut self,
params: &P,
domain: &EvaluationDomain<C::Scalar>,
p: &Argument,
) -> VerifyingKey<C> {
self.build_ordered_mapping();
build_vk(params, domain, p, |i, j| self.mapping_at_idx(i, j))
}

pub(crate) fn build_pk<'params, C: CurveAffine, P: Params<'params, C>>(
&mut self,
params: &P,
domain: &EvaluationDomain<C::Scalar>,
p: &Argument,
) -> ProvingKey<C> {
self.build_ordered_mapping();
build_pk(params, domain, p, |i, j| self.mapping_at_idx(i, j))
}

/// Returns columns that participate in the permutation argument.
pub fn columns(&self) -> &[ColumnMid] {
&self.columns
}

/// Returns mappings of the copies.
pub fn mapping(
&self,
) -> impl Iterator<Item = impl IndexedParallelIterator<Item = (usize, usize)> + '_> {
(0..self.num_cols).map(move |i| {
(0..self.col_len)
.into_par_iter()
.map(move |j| self.mapping_at_idx(i, j))
})
}
}
*/

pub(crate) fn build_pk<'params, C: CurveAffine, P: Params<'params, C>>(
params: &P,
domain: &EvaluationDomain<C::Scalar>,
Expand Down
4 changes: 4 additions & 0 deletions halo2_frontend/src/circuit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ pub fn compile_circuit<F: Field, ConcreteCircuit: Circuit<F>>(
let selector_polys = selectors_to_fixed.convert(assembly.selectors);
fixed.extend(selector_polys);

// sort the "copies" for deterministic ordering
#[cfg(feature = "thread-safe-region")]
assembly.permutation.copies.sort();

let preprocessing = PreprocessingV2 {
permutation: halo2_middleware::permutation::AssemblyMid {
copies: assembly.permutation.copies,
Expand Down
4 changes: 2 additions & 2 deletions halo2_frontend/src/dev.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,8 +751,8 @@ impl<F: FromUniformBytes<64> + Ord> MockProver<F> {
v
}));

// #[cfg(feature = "thread-safe-region")]
ed255 marked this conversation as resolved.
Show resolved Hide resolved
// prover.permutation.build_ordered_mapping();
#[cfg(feature = "thread-safe-region")]
prover.permutation.copies.sort();

Ok(prover)
}
Expand Down
2 changes: 1 addition & 1 deletion halo2_middleware/src/circuit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ impl fmt::Display for ColumnMid {
}

/// A cell identifies a position in the plonkish matrix identified by a column and a row offset.
#[derive(Clone, Debug)]
#[derive(Clone, PartialEq, PartialOrd, Eq, Ord, Debug)]
pub struct Cell {
pub column: ColumnMid,
pub row: usize,
Expand Down
Loading