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.

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

implement DEEP-ALI #190

Merged
merged 21 commits into from
Apr 21, 2023
Merged

implement DEEP-ALI #190

merged 21 commits into from
Apr 21, 2023

Conversation

jan-ferdinand
Copy link
Member

No description provided.

@jan-ferdinand jan-ferdinand marked this pull request as ready for review April 20, 2023 13:15
Copy link
Collaborator

@aszepieniec aszepieniec left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -182,7 +186,8 @@ impl Stark {
prof_start!(maybe_profiler, "ext tables");
prof_start!(maybe_profiler, "LDE");
master_ext_table.randomize_trace();
let fri_domain_ext_master_table = master_ext_table.to_fri_domain_table();
let (fri_domain_ext_master_table, ext_interpolation_polys) =
master_ext_table.to_fri_domain_table();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need to keep track of the polynomials if we have barycentric evaluation. File under: optimizable-if-bottleneck.

prof_start!(maybe_profiler, "out-of-domain rows");
let trace_domain_generator = derive_domain_generator(padded_height.value());
let out_of_domain_value_curr_row = proof_stream.sample_scalars(1)[0];
let out_of_domain_value_next_row = trace_domain_generator * out_of_domain_value_curr_row;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest to use "point" instead of "value" because "value" implies (to my ears at least) that it's the output of some evaluation. Here's it's the opposite: it's the input!

&base_and_ext_codeword,
quotient_domain,
out_of_domain_value_next_row,
out_of_domain_next_row_base_and_ext_element,
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer to avoid overloading the term "quotient" because then there will be quotient quotients.

prof_start!(maybe_profiler, "out-of-domain quotient element");
prof_start!(maybe_profiler, "zerofiers");
let one = BFieldElement::one();
let trace_domain_generator = derive_domain_generator(padded_height as u64);
Copy link
Collaborator

Choose a reason for hiding this comment

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

let trace_domain_generator = derive_domain_generator(padded_height as u64);

Did I read that line twice?

.par_iter()
.map(StarkHasher::hash)
.map(|&x| x.into())
.collect::<Vec<_>>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what these three lines are supposed to do.

let revealed_quotient_digests = revealed_quotient_elements
            .par_iter()
            .map(|&x| x.into())

Maybe add an explicit type to clarify?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have a added the following comment instead:

// Interpret the leaves, which are XFieldElements, as Digests, without hashing.

Does that clarify the operation sufficiently?

jan-ferdinand and others added 19 commits April 21, 2023 13:44
- don't hash codewords, interpret `XFieldElement`s as `Digest`s directly
- don't return `Result<…>` where only `Ok(…)` is ever used
- `fri.verify()` checks first codeword's Merkle root implicitly
- `fri.verify()` returns revealed indices and elements on success
- remove now-obsolete `FriValidationError` variant
- improve comments & code readability
Use the codeword and the indices from the first round of FRI instead,
which is identical anyway.
Co-authored-by: Alexander Lemmens <alexanderlemmens@hotmail.com>
@jan-ferdinand jan-ferdinand merged commit 9606441 into master Apr 21, 2023
@jan-ferdinand jan-ferdinand deleted the deep branch April 21, 2023 16:07
jan-ferdinand added a commit that referenced this pull request Apr 24, 2023
- do DEEP-ALI instead of plain ALI in the zk-STARK (#190) (9606441)
- add convenience functions for using Triton VM (0dab32a) (dda05e4)
- improve Triton profiler (#191) (7edd1a2)
  - add profiling of categories
  - introduce more weights for relative runtime costs
  - print relative time for all tasks and sub-tasks
- make method `debug` more powerful (ab49df7)
  - add optional argument: initial state
  - add optional argument: number of execution steps
- add construction of AET (witness generation) to profiling (c6e7b1e)
- use `cfg(debug_assertions)`, not environment variable (b0052f1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants