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

use quotient domain instead of fri domain wherever applicable #124

Merged
merged 11 commits into from
Nov 18, 2022
Merged

use quotient domain instead of fri domain wherever applicable #124

merged 11 commits into from
Nov 18, 2022

Conversation

jan-ferdinand
Copy link
Member

Fix #114

sshine
sshine reviewed Nov 15, 2022
View reviewed changes
Copy link
Collaborator

@sshine sshine left a comment

Choose a reason for hiding this comment

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

Great work.

As I am not qualified to assess the math in #114, I am only commenting on the software part.

I don't have an objection to the name "arithmetic domain", but I wonder if all mathematical domains aren't arithmetic in some sense? It might not be a bad name because anyone who dug this deep will know that it must mean something more than that it contains numbers.

triton-vm/src/domain.rs Outdated Show resolved Hide resolved
triton-vm/src/domain.rs Outdated Show resolved Hide resolved
triton-vm/src/cross_table_arguments.rs Outdated Show resolved Hide resolved
triton-vm/src/domain.rs Outdated Show resolved Hide resolved
triton-vm/src/domain.rs Outdated Show resolved Hide resolved
triton-vm/src/domain.rs Outdated Show resolved Hide resolved
triton-vm/src/table/table_collection.rs Outdated Show resolved Hide resolved
@jan-ferdinand
Copy link
Member Author

I don't have an objection to the name "arithmetic domain", but I wonder if all mathematical domains aren't arithmetic in some sense?

You are probably right. The reason for the name is its purpose. As of this PR, the STARK prover performs as much arithmetic as possible over the – well – arithmetic domain. In particular, this means computing the quotients, and any intermediate steps to get the quotients. The (bigger) FRI domain is only used for anything related to FRI. The transition from arithmetic domain to FRI domain happens after the combination codeword has been computed.

I'm happy to use a different name. I think neither “quotient domain” nor “combination domain” good names. Maybe someone else has a good idea?

@aszepieniec
Copy link
Collaborator

I like "quotient domain" and would propose to repurpose the term "arithmetic domain" to denote what is currently being called "domain".

@jan-ferdinand jan-ferdinand force-pushed the arithmetic_domain branch 2 times, most recently from 4829f33 to 9a6343c Compare November 16, 2022 22:46
@jan-ferdinand jan-ferdinand changed the title use arithmetic domain instead of fri domain wherever applicable use quotient domain instead of fri domain wherever applicable Nov 16, 2022
aszepieniec
aszepieniec requested changes Nov 17, 2022
View reviewed changes
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.

Looks good to me. Minor changes requested before merging. Major requested changes requested in #128 .

triton-vm/src/cross_table_arguments.rs Outdated Show resolved Hide resolved
triton-vm/src/cross_table_arguments.rs Outdated Show resolved Hide resolved
triton-vm/src/stark.rs Show resolved Hide resolved
triton-vm/src/stark.rs Show resolved Hide resolved
triton-vm/src/stark.rs Show resolved Hide resolved
triton-vm/src/table/extension_table.rs Show resolved Hide resolved
triton-vm/src/table/extension_table.rs Show resolved Hide resolved
triton-vm/src/table/extension_table.rs Show resolved Hide resolved
triton-vm/src/table/extension_table.rs Outdated Show resolved Hide resolved
triton-vm/src/table/extension_table.rs Outdated Show resolved Hide resolved
@aszepieniec
Copy link
Collaborator

One more consideration: maybe pure LDE should live on ArithmeticDomain. Possibly randomized LDE also.

@jan-ferdinand jan-ferdinand mentioned this pull request Nov 17, 2022
Closed
@jan-ferdinand
Copy link
Member Author

One more consideration: maybe pure LDE should live on ArithmeticDomain. Possibly randomized LDE also.

I think that's a good idea. We could provide LDE and dual LDE, and accept one (respectively, two) target domains, as well as a number of randomizers to be inserted. If that value is set to 0, pure (dual) LDE is automatically performed.

@jan-ferdinand
Copy link
Member Author

One more consideration: maybe pure LDE should live on ArithmeticDomain. Possibly randomized LDE also.

It turns out that is has the same difficulty as removing the generic type from ArithmeticDomain. I'll move LDE once #127 is complete.

jan-ferdinand and others added 11 commits November 18, 2022 11:32
@jan-ferdinand
rename FriDomain to Domain, drop unnecessary parameter for lde
8657d8d
@jan-ferdinand
use optimally sized subgroup for fast interpolation of columns
a14fd83 
Remove superfluous parameter `fri_domain`.
@jan-ferdinand
only ever interpolate and evaluate over Domains of BFieldElements
b3e926e 
Only accept generators of type `BFieldElement`, not any Finite Field.
Remove duplicate FRI domain in struct `Stark`.
@jan-ferdinand
perform low-degree extension over two domains: arithmetic and fri (#114)
199ada1
@jan-ferdinand
use arithmetic domain instead of fri domain wherever applicable
5562230 
Fix #114
@jan-ferdinand
use trace_domain_generator instead of omicron
f9c1e95
@jan-ferdinand
use fri_domain_generator instead of omega
4b7787d
@jan-ferdinand
drop parameter for trace_domain_generator, generate it where needed
f56e70e
@jan-ferdinand
rename arithmetic_domain to quotient_domain
083abf8
@jan-ferdinand
rename Domain to ArithmeticDomain
1705047
@jan-ferdinand @aszepieniec
incorporate Alan's feedback
465d42d 
- consistently name parameters `quotient_domain`
- use `prof_start` & `prof_stop` macros consistently

Co-authored-by: Alan <alan@neptune.cash>
@jan-ferdinand jan-ferdinand merged commit 776fa19 into master Nov 18, 2022
@jan-ferdinand jan-ferdinand deleted the arithmetic_domain branch November 18, 2022 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sshine sshine sshine left review comments

@aszepieniec aszepieniec aszepieniec approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

compute quotient codewords more efficiently
3 participants
@jan-ferdinand @aszepieniec @sshine