-
Notifications
You must be signed in to change notification settings - Fork 38
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
Parallelize test_only segment simulation #498
Conversation
zero_bin/ops/src/lib.rs
Outdated
let _ = SegmentDataIterator::<Field>::new(&inputs.0, Some(inputs.1)).collect::<Vec<_>>(); | ||
|
||
Ok(()) |
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.
let _ = SegmentDataIterator::<Field>::new(&inputs.0, Some(inputs.1)).collect::<Vec<_>>(); | |
Ok(()) | |
simulate_execution_all_segments::<Field>(&inputs.0, inputs.1) |
zero_bin/ops/src/lib.rs
Outdated
@@ -25,6 +28,22 @@ use zero_bin_common::{debug_utils::save_inputs_to_disk, prover_state::p_state}; | |||
|
|||
registry!(); | |||
|
|||
#[cfg(feature = "test_only")] | |||
#[derive(Deserialize, Serialize, RemoteExecute)] | |||
pub struct BatchTestOnly {} |
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.
is there any reason why you're not using save_inputs_on_error
field in this? Even though we technically can't log yet the inputs upon error because the iterator is panicking, we are aiming at changing this logic before the release, which would allow to save inputs even in testing mode.
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.
Yeah, I assumed we would add it when logging is available but we can have it there already. I also mimicked the error handling of the other operations for the future.
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.
Thanks! LGTM!
* Parallelize test_only segment simulation * Address comments
Closes #390.