[feat] support custom segmentation strategy #1245
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
luffykai
commented
Jan 21, 2025
| @@ -27,6 +29,40 @@ use crate::{ | |||
| /// Check segment every 100 instructions. | |||
| const SEGMENT_CHECK_INTERVAL: usize = 100; | |||
|
|
|||
| const DEFAULT_MAX_SEGMENT_LEN: usize = (1 << 22) - 100; | |||
crates/vm/src/arch/segment.rs
Outdated
| const DEFAULT_MAX_CELLS_PER_CHIP_IN_SEGMENT: usize = DEFAULT_MAX_SEGMENT_LEN * 120; | ||
|
|
||
| pub trait SegmentationStrategy { | ||
| fn should_segment(&self, trace_heights: &[usize], trace_cells: &[usize]) -> bool; |
There was a problem hiding this comment.
i think it's more general to take the chip_complex as the input, but that has some generic types and I wasn't able to get it work with Arc<dyn>.
Do we think there are other information needed to determine if one should segment?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
luffykai
commented
Jan 22, 2025
| @@ -59,6 +59,16 @@ impl<VC> ContinuationProver<VC> { | |||
| self | |||
| } | |||
|
|
|||
| pub fn set_max_segment_len(&mut self, max_segment_len: usize) -> &mut Self { | |||
| self.stark_prover | |||
There was a problem hiding this comment.
should refactor somehow
This comment has been minimized.
This comment has been minimized.
luffykai
commented
Jan 22, 2025
| @@ -67,8 +67,7 @@ impl NativeConfig { | |||
| ..Default::default() | |||
| }, | |||
| num_public_values, | |||
| ) | |||
| .with_max_segment_len((1 << 24) - 100), | |||
There was a problem hiding this comment.
hmm i am not sure where should put this.
It might be more ideal for this segment strategy to be vm_config so all the "config values" are in one place... but couldn't not make it serde
Commit: 7bf930d |
|
for #1254 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
based on discussion from #1217
fix INT-3005