Add FRI comparison, closes issue #279 #366

Vindaar · 2025-11-21T15:36:21Z

This PR adds a comparison to the FRI PCS in the main binary. Due to the very different nature of FRI vs WHIR (multilinear vs univariate), I'm not certain how apples-to-apples this really is.

In addition I still have a few open questions:

pretty uncertain if I should use TwoAdicMultiplicativeCoset or some other domain?
do I handle the RowMajorMatrix construction sensibly? I split the evaluations sampled for WHIR into NUM_COLS. Currently a const, but we could make that a command line argument.
The two adicity of KoalaBear is 24. This means the longest "trace height" supported is 2^23. The default parameters sets num_variables to 25. At NUM_COLS = 1, this would fail.
I'm uncertain about how I construct the verifier points to be honest.
currently we don't use a packing variant of Poseidon2. Should we?

With the parameters I currently hardcoded, I get this output:

Proving time: 1694 ms (commit: 786 ms, opening: 908 ms)
proof size: 106.95 KiB
Verification time: 2636 μs

=========================================
FRI (PCS) 🍳️
.........
Proof Verified Successfully

Proving time: 1059 ms (commit: 641 ms, opening: 418 ms)
proof size: 315.26 KiB
Verification time: 8199 μs

Next steps are adapting it to use the WHIR parameters and reduce it to only the FRI PCS. The code in this commit is simply ripped out of the Plonky3 examples, where they build an entire STARK example.

WizardOfMenlo · 2025-11-21T19:27:19Z

Not sure how hard it is to do, but if you want to compare FRI and WHIR apple to apple (as low-degree tests) you can remove the following from WHIR (either properly or just from the timings)

The OOD sample in the commit phase.
The initial sumcheck in WHIR (basically, you only require a sumcheck after the first prover oracle is sent).

(You can look at https://github.com/WizardOfMenlo/whir/blob/15cf6668e904ed2e80c9e6209dcce69f5bcf79b9/src/bin/main.rs#L195 for an example, I think it's the initial_statement: false option therein).

tcoratger · 2025-11-23T20:08:06Z

@WizardOfMenlo @Vindaar

Yes I think that right now WHIR is running as a Polynomial Commitment Scheme while FRI is running as a Low-Degree Test.

A Low-Degree Test verifies that a function $f: L \to \mathbb{F}$ is close to a low-degree polynomial. So in this case, inputs are evaluations of $f$ over domain $L$ and the verifier accept if $f$ is $\delta$-close to a degree-$d$ polynomial, reject otherwise. But there is no evaluation constraints, it only tests proximity to the code.

On the other hand, a PCS allows a prover to commit to a polynomial and later open it at specific points. So, the prover commits to polynomial $f$ and later prover reveals $f(z) = y$ for verify-chosen point $z$. So it uses LDT as a building block but adds evaluation checking.

The fact that you have initial_statement: true is what makes WHIR run as PCS, not LDT. It includes an initial sumcheck to verify evaluations and adds out-of-domain (OOD) checks during commit.

But indeed, by marking this flag as false (doesn't check the evaluation constraints), you will avoid the first sumcheck and thus save significant costs.

I believe you can take inspiration from the link sent by Giacomo on the other WHIR repo, but I think it's desirable to keep a main run with WHIR as PCS in parallel (in addition to the comparison with FRI) because that's what interests us most. Therefore, to get an idea of the performance improvements, it's always useful to be able to run it as PCS quickly with the CLI.
It also seems that you have a much higher security level for WHIR. The number of queries for FRI (40) and the 8 bits of grinding seems much lower than the 90 bits of security imposed by default in the WHIR cli no? So in my sense, with the current configuration, FRI requires fewer queries, smaller proofs and faster verification due to this mismatch.

We should probably have something like this to match (not sure about my calculations, we need to make this clean and document where the calculations come from so that the reader is sure to understand):
```
  let target_security = 90;
  let fri_queries = target_security - pow_bits;
  
  FriParameters {
      log_blowup: 1,
      num_queries: fri_queries, 
      proof_of_work_bits: pow_bits,
      // ...
  }
```
The matrix reshaping thing looks a bit strange to me. If I understand correctly:
1. We take a univariate polynomial with 2^25 evaluations
2. We arbitrarily split it into 32 columns
3. Reduces domain size from 2^25 to 2^20
4. Creates a 2^20 × 32 matrix
Why 32 columns? It seems that the domain size 2^20 instead of 2^25 changes the problem, so it's not exactly clear to me why we're doing this. I'd like to be sure that by reshaping the problem in this way, we're not changing the conditions and therefore comparing apples to apples (maybe if we just do const NUM_COLS: usize = 1; that's enough, right?).

I also think we're using evaluation point types that aren't comparable with

let points: Vec<_> = (0..ctx.num_evaluations)
    .map(|_| MultilinearPoint::rand(&mut ctx.rng, params.num_variables))
    .collect();

let open_points: Vec<EF> = (0..ctx.num_evaluations)
    .map(|_| ctx.rng.random::<EF>())
    .collect();

But migrating to the LDT comparison should remove this because in this case, we're not doing evaluation checks at all; we're just testing proximity.

I also think the comparison of proof sizes isn't exactly fair.
- WHIR: Calculates from field elements: num_elements * bits_per_element / 8
- FRI: Uses bincode serialization which adds overhead (length prefixes, metadata)
It's minor but I think that in the actual comparison, it is not exactly clear what the relationship is between WHIR's multilinear degree and FRI's univariate degree is not explicit.

WHIR:
- num_variables = 25
- Multilinear polynomial: $\hat{f}(x_1, \ldots, x_{25})$
- Degree: 1 in each variable
- Total evaluations: $2^{25}$
- Can be viewed as a univariate of degree $2^{25} - 1$
FRI (current setup):
- Domain: $2^{20}$ points with 32 columns
- Unclear what the actual univariate degree being tested is
We should document this more clearly in the code so the reader can understand exactly how we are trying to compare univariate and multilinear functions.

tcoratger · 2025-11-23T20:10:21Z

src/bin/main.rs

+    println!("Verification time: {} μs", verify_time.as_micros());
+}
+
+#[allow(clippy::too_many_lines)]


Suggested change

#[allow(clippy::too_many_lines)]

SyxtonPrime

LGTM

As far as I can tell, you've set up FRI correctly so I think this is a reasonably fair test on that side.

SyxtonPrime · 2025-11-25T17:07:14Z

src/bin/main.rs

+pub const fn create_benchmark_fri_params<Mmcs>(mmcs: Mmcs) -> FriParameters<Mmcs> {
+    FriParameters {
+        log_blowup: 1,
+        log_final_poly_len: 0,
+        num_queries: 40,
+        proof_of_work_bits: 8,
+        mmcs,
+    }
+}


Ideally, we should fix a security parameter and then calculate the number of queries based off that.

But with target_security = 90 for WHIR vs num_queries: 40 here for FRI we don't have the same level of security no? Or did I miss something?

Yeah, I should have mentioned that these are obviously placeholder at the moment. I just don't have a good intuition for how to choose parameters sensibly here to compare with WHIR.

SyxtonPrime · 2025-11-25T17:09:25Z

src/bin/main.rs

+    // Define the number of columns we split the evaluations into
+    // TODO: could make this a CL arg?
+    const LOG_NUM_COLS: usize = 5;
+    const NUM_COLS: usize = 1 << LOG_NUM_COLS; // 32


Hmm this doesn't really make sense from the FRI perspective (i.e. the low degree test).

I guess this is like doing several low degree tests in parallel?

SyxtonPrime · 2025-11-25T17:13:24Z

src/bin/main.rs

+
+    // Commit to the matrix
+    let commit_time = Instant::now();
+    let (commitment, prover_data) = Pcs::<EF, MyChallenger>::commit(&pcs, matrix_iter.clone());


This is cleaner

Suggested change

let (commitment, prover_data) = Pcs::<EF, MyChallenger>::commit(&pcs, matrix_iter.clone());

let (commitment, prover_data) = pcs.commit(matrix_iter.clone());

Yeah, that's what I wrote initially, but it leads to fun type annotations needed errors:

error[E0284]: type annotations needed --> src/bin/main.rs:384:41 | 384 | let (commitment, prover_data) = pcs.commit(matrix_iter.clone()); | ^^^^^^ cannot infer type for type parameter `Challenger` | = note: cannot satisfy `<_ as GrindingChallenger>::Witness == p3_monty_31::monty_31::MontyField31<KoalaBearParameters>` = note: required for `TwoAdicFriPcs<MontyField31<KoalaBearParameters>, Radix2DFTSmallBatch<MontyField31<KoalaBearParameters>>, MerkleTreeMmcs<..., ..., ..., ..., 8>, ...>` to implement `Pcs<BinomialExtensionField<p3_monty_31::monty_31::MontyField31<KoalaBearParameters>, 4>, _>` = note: the full name for the type has been written to '/home/basti/src/rust/whir-p3/target/debug/deps/main-6703cdc4c79f20a7.long-type-9852739075644608931.txt' = note: consider using `--verbose` to print the full type name to the console error[E0283]: type annotations needed --> src/bin/main.rs:384:41 | 384 | let (commitment, prover_data) = pcs.commit(matrix_iter.clone()); | ^^^^^^ cannot infer type for type parameter `Challenger` | = note: multiple `impl`s satisfying `_: FieldChallenger<p3_monty_31::monty_31::MontyField31<KoalaBearParameters>>` found in the `p3_challenger` crate: ................................... error[E0283]: type annotations needed --> src/bin/main.rs:384:41 | 384 | let (commitment, prover_data) = pcs.commit(matrix_iter.clone()); | ^^^^^^ cannot infer type for type parameter `Challenger` | = note: multiple `impl`s satisfying `_: CanObserve<p3_symmetric::Hash<p3_monty_31::monty_31::MontyField31<KoalaBearParameters>, p3_monty_31::monty_31::MontyField31<KoalaBearParameters>, 8>>` found in the `p3_challenger` crate: ..................................

at which point I just resigned to calling it as above...

SyxtonPrime · 2025-11-25T17:13:51Z

src/bin/main.rs

+    let (opened_values, proof) = Pcs::<EF, MyChallenger>::open(
+        &pcs,
+        vec![(&prover_data, points)],
+        &mut ctx.challenger.clone(),
+    );


Suggested change

let (opened_values, proof) = Pcs::<EF, MyChallenger>::open(

&pcs,

vec![(&prover_data, points)],

&mut ctx.challenger.clone(),

);

let (opened_values, proof) = pcs.open(

vec![(&prover_data, points)],

&mut ctx.challenger.clone(),

);

Huh, this does not actually produce the same problems as for commit. I thought I saw similar errors, but I guess that was while I was still writing code or I misremember.

SyxtonPrime · 2025-11-25T17:14:16Z

src/bin/main.rs

+    let res = Pcs::<EF, MyChallenger>::verify(
+        &pcs,
+        vec![(commitment, verifier_points)],
+        &proof,
+        &mut ctx.challenger.clone(),
+    );


Suggested change

let res = Pcs::<EF, MyChallenger>::verify(

&pcs,

vec![(commitment, verifier_points)],

&proof,

&mut ctx.challenger.clone(),

);

let res = pcs.verify(

vec![(commitment, verifier_points)],

&proof,

&mut ctx.challenger.clone(),

);

Also works to my surprise.

SyxtonPrime · 2025-11-25T17:16:27Z

Also I'm pretty sure you are using the packing version of Poseidon2. This should be used automatically due to how you've defined Poseidon2MerkleMmcs.

Vindaar · 2025-11-28T14:26:22Z

First of all sorry for being absent. Spent the majority of the week in bed.

So before I address some of your comments explicitly, let me show my ignorance.

You mention that in the FRI code here we use FRI only as a low degree test. I was under the impression that the TwoAdicFriPcs performs a FRI PCS and not just FRI the low degree test. So that understanding is wrong?

Regarding the multi column approach: I did that for three reasons:

as mentioned at the default value of 25 for num_variables leads to a trace height of 2^25, which is too large for KoalaBears's two adicity. I assumed the fact that WHIR doesn't run into trouble here means it also internally dealt with a domain size less than 2^24. For example params.max_fft_size() for WHIR returns 21 with the default parameters. That to me implied we were not dealing with a 2^25 element domain either in WHIR.
WHIR is multilinear. In my (maybe wrong) understanding that means we are committing to multiple columns "by design" encoded as a multilinear polynomial. If we only commit to a single column, we effectively commit to a univariate polynomial, no? My naive understanding was that the MultilinearPoint::rand(&mut ctx.rng, params.num_variables) would construct multilinear points with num_variables (25 by default).
I assumed that the main thing we want to compare is commiting to the same number of coefficients with both PCSs. How they are laid out (single or multiple columns) in some sense is less important initially. Obviously we'd need to decide on what we want to compare in the end. Further, the polynomial we construct is constructed with 1 << num_variables coefficients. Given that the MultilinearPoints are constructed with num_variables, I assumed this would be essentially mean roughly num_variables columns with (1 << num_variables) / num_variables rows, if interpreting the multilinear polynomials that way. And thus 32 was just the closest power of two to turn into a RowMajorMatrix.

I guess that understanding is completely wrong then? I suppose your @tcoratger

Can be viewed as a univariate of degree 2^{25} - 1

addresses parts of my misunderstanding. My long breaks working on other stuff since initially reading up on WHIR and the related concepts is certainly not helping. 🫣

Also I'm pretty sure you are using the packing version of Poseidon2. This should be used automatically due to how you've defined Poseidon2MerkleMmcs.

Oh, right. I thought part of my last minute removals of a bunch of leftovers from the entire STARK setup, I also removed this custom MerkleTreeMmcs definition.

FRI: Uses bincode serialization which adds overhead (length prefixes, metadata)

True. For the moment I just assumed the overhead would be small, but that may be wrong.

Vindaar added 3 commits November 21, 2025 16:27

add initial FRI version (copy of Plonky3 FRI example)

bb33e5d

Next steps are adapting it to use the WHIR parameters and reduce it to only the FRI PCS. The code in this commit is simply ripped out of the Plonky3 examples, where they build an entire STARK example.

fix FRI comparison to only compare FRI PCS

a011349

clean up dependencies & imports, reset num_variables default to 25

f1de8f4

tcoratger reviewed Nov 23, 2025

View reviewed changes

SyxtonPrime reviewed Nov 25, 2025

View reviewed changes

Vindaar added 2 commits November 28, 2025 14:03

remove too_many_lines from main fn which is now short

c100dee

use pcs.open, pcs.verify instead of call via Pcs::...

80db7bf

	let (commitment, prover_data) = Pcs::<EF, MyChallenger>::commit(&pcs, matrix_iter.clone());
	let (commitment, prover_data) = pcs.commit(matrix_iter.clone());

Add FRI comparison, closes issue #279 #366

Are you sure you want to change the base?

Add FRI comparison, closes issue #279 #366

Uh oh!

Conversation

Vindaar commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WizardOfMenlo commented Nov 21, 2025

Uh oh!

tcoratger commented Nov 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SyxtonPrime left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SyxtonPrime commented Nov 25, 2025

Uh oh!

Vindaar commented Nov 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Vindaar commented Nov 21, 2025 •

edited

Loading