-
Notifications
You must be signed in to change notification settings - Fork 121
Empirical constraints suggestions #3468
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.
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
base: collect-empirical-constraints
Are you sure you want to change the base?
Empirical constraints suggestions #3468
Conversation
| /// (most of the time), but are not guaranteed to hold in all cases. | ||
| #[derive(Serialize, Deserialize, Clone, Default, Debug)] | ||
| #[derive(Serialize, Default, Debug)] | ||
| pub struct EmpiricalConstraints { |
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.
Merge DebugInfo into EmpiricalConstraints to simplify return types
| } | ||
|
|
||
| #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize, Debug, Copy, Clone)] | ||
| pub struct BlockCell { |
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.
Introduce type for the equivalence class members, aka cells of a block execution
| /// - It cannot be empty | ||
| /// - It cannot hold a single element |
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.
Instead of filtering out equivalence classes, keep them out by construction
| Id: Eq + Hash + Copy + Ord, | ||
| { | ||
| pub fn intersect_partitions<T: Eq + Hash + Copy + Ord>( | ||
| partitions: Vec<EquivalenceClasses<T>>, |
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.
move
openvm/src/empirical_constraints.rs
Outdated
| } | ||
| let main = proving_context.common_main.as_ref().unwrap(); | ||
| assert_eq!(main.width, column_names.len()); | ||
| let (symbolic_machine, _) = airs.machine_by_insertion_idx.get(air_id).unwrap(); |
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.
With this change to the extraction, we can access the instruction machine by air_id, so we can run the analysis on SymbolicMachine which is cleaner than on the column names.
| } | ||
|
|
||
| impl<'a> ConcreteBlock<'a> { | ||
| fn equivalence_classes(&self) -> EquivalenceClasses<BlockCell> { |
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.
Moved out of ConstraintDetector into this implementation
A collection of suggestions on top of #3461