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

Jump to bottom

Cell manager context #17

Open
wants to merge 2 commits into
base: circuit-tools-wip
Choose a base branch
from
Open

Cell manager context #17

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2addd74
cm with ctx
CeciliaZ030 Jun 29, 2023
7cbb5b9
update
CeciliaZ030 Jul 5, 2023
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
95 changes: 90 additions & 5 deletions zkevm-circuits/src/circuit_tools/cell_manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
},
poly::Rotation,
};
use std::{cmp::Ordering, collections::BTreeMap, fmt::Debug, hash::Hash};
use lazy_static::__Deref;
use std::{collections::{BTreeMap, HashMap}, fmt::Debug, hash::Hash, cmp::{max, Ordering}};

#[derive(Clone, Debug, Default)]
pub(crate) struct Cell<F> {
Expand Down Expand Up @@ -208,11 +209,10 @@
pub(crate) expr: Expression<F>,
}


impl<F: Field, C: CellType> PartialEq for CellColumn<F, C> {
fn eq(&self, other: &Self) -> bool {
self.index == other.index
&& self.cell_type == other.cell_type
&& self.height == other.height
self.index == other.index && self.cell_type == other.cell_type && self.height == other.height
}
}

Expand All @@ -236,12 +236,25 @@
}
}


#[derive(Clone, Debug)]
pub struct CellManager<F, C: CellType> {
configs: Vec<CellConfig<C>>,
columns: Vec<CellColumn<F, C>>,
height: usize,
width: usize,
height_limit: usize,

// branch ctxs
branch_ctxs: HashMap<String, CmContext<F, C>>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it a hashmap and not a simple vector? Contexts will work like a stack no? Similarly to how we handle the condition in the constraint builder. So if you enter a context you just push a new context on the stack, if you exit a context you just pop it from the stack?

Copy link
Owner Author

@CeciliaZ030 CeciliaZ030 Jul 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brechtpd#9 If you scrolled down to one of my replies:

I thought of it and figured that a dynamic branching callstack is equivalent to a static upside-down tree:

   |_if_| |_else_|     |_if_|
  |_____if_______|    |_else_|
 |__________________________|

A simple callstack structure won't work cuz we need to remember what happened in the past. Thus we have to save the entire tree.

In the condition case remember we had to use region_id to hack around this problem, basically manually divide the branching callstack into states labeled with id:

   |_if_| |_else_|     |_if_|
  |___ match____|    |_match__|
 |______________________________|
 (      START     )(   ACCOUNT   ) ....

And one time i said ideally we can make store expressions work like CM ctx by changing the macros, but I failed 😭 and that could be something we can do for the next iteration.

parent_ctx: Option<CmContext<F, C>>,
}


#[derive(Default, Clone, Debug)]
struct CmContext<F, C: CellType>{
parent: Box<Option<CmContext<F, C>>>,
columns: Vec<CellColumn<F, C>>,
}

impl<F: Field, C: CellType> CellManager<F, C> {
Expand All @@ -255,7 +268,8 @@
.into_iter()
.map(|c| c.into())
.collect::<Vec<CellConfig<C>>>();


let mut width = 0;
let mut columns = Vec::new();
for config in configs.iter() {
let cols = config.init_columns(meta);
Expand All @@ -274,16 +288,87 @@
expr: cells[0].expr(),
cells,
});
width += 1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

width is just columns.len()? Why the extra variable?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably because in previous version there was a width? I'll get rid of it :)

}
}
Self {
configs,
columns,
height: max_height,
width,
height_limit: max_height,
branch_ctxs: HashMap::new(),
parent_ctx: None,
}
}

pub(crate) fn cur_to_parent(&mut self) {
let new_parent = match self.parent_ctx.clone() {
// if parent context exists, meaning we are deep in a callstack
// we set it as the parent of new parent
Some(ctx) => CmContext {
parent: Box::new(Some(ctx.clone())),
columns: self.columns.clone(),
},
// otherwise, this is the fist level of callstack
// the parent of new parent is None
None => CmContext {
parent: Box::new(None),
columns: self.columns.clone(),
}
};
self.parent_ctx = Some(new_parent);
self.reset(self.height_limit);
}

pub(crate) fn cur_to_branch(&mut self, name: &str) {
let new_branch = match self.parent_ctx.clone() {
// if parent context exists, meaning we are deep in a callstack
// we set it as the parent of new branch
Some(ctx) => CmContext {
parent: Box::new(Some(ctx.clone())),
columns: self.columns.clone(),
},
// otherwise, this is the fist level of callstack
// the parent of new branch is None
None => CmContext {
parent: Box::new(None),
columns: self.columns.clone(),
}
};
self.branch_ctxs.insert(name.to_string(), new_branch);
self.reset(self.height_limit);
}

pub(crate) fn recover_max_branch(&mut self) {
let mut new_cols = self.columns.clone();
let parent = self.parent_ctx.clone().expect("Retruning context needs parent");
self.branch_ctxs
.iter()
.for_each(|(name, ctx)| {

Check warning on line 348 in zkevm-circuits/src/circuit_tools/cell_manager.rs

View workflow job for this annotation

GitHub Actions / Rustfmt 

unused variable: `name`

Check warning on line 348 in zkevm-circuits/src/circuit_tools/cell_manager.rs

View workflow job for this annotation

GitHub Actions / Intra-doc links 

unused variable: `name`

Check warning on line 348 in zkevm-circuits/src/circuit_tools/cell_manager.rs

View workflow job for this annotation

GitHub Actions / Bitrot check 

unused variable: `name`

Check warning on line 348 in zkevm-circuits/src/circuit_tools/cell_manager.rs

View workflow job for this annotation

GitHub Actions / Build target wasm32-wasi 

unused variable: `name`

Check warning on line 348 in zkevm-circuits/src/circuit_tools/cell_manager.rs

View workflow job for this annotation

GitHub Actions / Build target wasm32-wasi 

unused variable: `name`

Check warning on line 348 in zkevm-circuits/src/circuit_tools/cell_manager.rs

View workflow job for this annotation

GitHub Actions / Build target wasm32-unknown-unknown 

unused variable: `name`
for c in 0..self.width {
new_cols[c] = max(&new_cols[c], &ctx.columns[c]).clone();
new_cols[c] = max(&new_cols[c], &parent.columns[c]).clone();
}
});
self.columns = new_cols;
self.branch_ctxs.clear();
self.parent_ctx = self.parent_ctx
.clone()
.map(|ctx| ctx.parent.deref().clone())
.unwrap();
}

pub(crate) fn recover_parent(&mut self) {
assert!(self.parent_ctx.is_some(), "No parent context to recover");
self.columns = self.parent_ctx.clone().unwrap().columns.clone();
self.parent_ctx
.clone()
.map(|ctx| self.parent_ctx = ctx.parent.deref().clone())
.unwrap();
self.branch_ctxs.clear();
}

pub(crate) fn query_cells(&mut self, cell_type: C, count: usize) -> Vec<Cell<F>> {
let mut cells = Vec::with_capacity(count);
while cells.len() < count {
Expand Down
Loading