Logup cumsum constraint with cumsum_shift

[shaharsamocha7]

No description provided.

[reviewable-StarkWare]

This change is 

[shaharsamocha7]

• Rename total_sum to claimed_sum #982
• Remove claimed_sum and LogupSums #979
• Logup cumsum constraint with cumsum_shift #978 👈 (View in Graphite)
• Remove padding examples #974
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 94.82759% with 3 lines in your changes missing coverage. Please review.

Project coverage is 92.02%. Comparing base (3ae4395) to head (b5c7e2a).

Files with missing lines	Patch %	Lines
crates/prover/src/constraint_framework/logup.rs	94.73%	1 Missing and 1 partial ⚠️
crates/prover/src/constraint_framework/mod.rs	91.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #978      +/-   ##
==========================================
- Coverage   92.33%   92.02%   -0.32%     
==========================================
  Files         105      105              
  Lines       14252    14274      +22     
  Branches    14252    14274      +22     
==========================================
- Hits        13159    13135      -24     
- Misses       1019     1064      +45     
- Partials       74       75       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ohad-starkware]

Reviewed 1 of 7 files at r1.
Reviewable status: 1 of 7 files reviewed, 4 unresolved discussions (waiting on @shaharsamocha7)

---

crates/prover/src/constraint_framework/logup.rs line 66 at r1 (raw file):

        interaction: usize,
        total_sum: SecureField,
        claimed_sum: Option<ClaimedPrefixSum>,

doc why this remains here when only valid value is None

---

crates/prover/src/constraint_framework/logup.rs line 72 at r1 (raw file):

        assert!(
            claimed_sum.is_none(),
            "Claimed sum at internal index is not supported"

Suggestion:

            "Partial prefix-sum is not supported"

---

crates/prover/src/constraint_framework/logup.rs line 195 at r1 (raw file):

        let mut last_col_coords = self.trace.pop().unwrap().columns;

        // compute cumsum_shift.

Suggestion:

  // Compute cumsum_shift.

---

crates/prover/src/constraint_framework/logup.rs line 201 at r1 (raw file):

        let base_sums = packed_sums.map(|s| s.pointwise_sum());
        let total_sum = SecureField::from_m31_array(base_sums);
        let cumsum_shift = total_sum / BaseField::from_u32_unchecked(1 << self.log_size);

?

Suggestion:

        let coordinate_sums = last_col_coords.each_ref().map(|c| {
            c.data
                .iter()
                .copied()
                .sum::<PackedBaseField>()
                .pointwise_sum()
        });
        let total_sum = SecureField::from_m31_array(coordinate_sums);
        let cumsum_shift = total_sum / BaseField::from_u32_unchecked(1 << self.log_size);

---

crates/prover/src/constraint_framework/logup.rs line 206 at r1 (raw file):

            c.data
                .iter_mut()
                .for_each(|x| *x -= PackedBaseField::broadcast(cumsum_shift.to_m31_array()[i]))

broadcast outside of inner loop, it will look nicer

---

crates/prover/src/constraint_framework/mod.rs line 186 at r1 (raw file):

            assert!(!self.logup.is_finalized, "LogupAtRow was already finalized");

            assert!(

will we ever get to this assert?

[ohad-starkware]

crates/prover/src/constraint_framework/logup.rs line 66 at r1 (raw file):

Previously, ohad-starkware (Ohad) wrote…

doc why this remains here when only valid value is None

nvm won't give you trouble on this

[ohad-starkware]

crates/prover/src/constraint_framework/mod.rs line 186 at r1 (raw file):

Previously, ohad-starkware (Ohad) wrote…

will we ever get to this assert?

same

[shaharsamocha7]

Reviewable status: 1 of 7 files reviewed, 2 unresolved discussions (waiting on @ohad-starkware)

---

crates/prover/src/constraint_framework/logup.rs line 66 at r1 (raw file):

Previously, ohad-starkware (Ohad) wrote…

nvm won't give you trouble on this

I delete it in the next pr.

---

crates/prover/src/constraint_framework/logup.rs line 206 at r1 (raw file):

Previously, ohad-starkware (Ohad) wrote…

broadcast outside of inner loop, it will look nicer

better?

---

crates/prover/src/constraint_framework/mod.rs line 186 at r1 (raw file):

Previously, ohad-starkware (Ohad) wrote…

same

next pr

---

crates/prover/src/constraint_framework/logup.rs line 195 at r1 (raw file):

        let mut last_col_coords = self.trace.pop().unwrap().columns;

        // compute cumsum_shift.

Done.

[ohad-starkware]

smar, ill start lgtming myself too

[ohad-starkware]

Reviewed 4 of 7 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @shaharsamocha7)

[ohad-starkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @shaharsamocha7)

[shaharsamocha7]

haha oops :)

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @shaharsamocha7)

[ohad-starkware]

Reviewed 3 of 3 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @shaharsamocha7)

[shaharsamocha7]

Merge activity

• Jan 15, 3:47 AM EST: A user started a stack merge that includes this pull request via Graphite.
• Jan 15, 3:49 AM EST: Graphite rebased this pull request as part of a merge.
• Jan 15, 3:53 AM EST: A user merged this pull request with Graphite.