seq in memory id->value

[ohad-starkware]

This change is 

[ohad-starkware]

• seq in memory id->value #411 👈 (View in Graphite)
• main

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

[shaharsamocha7]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @alon-f and @ohad-starkware)

---

stwo_cairo_prover/crates/prover/src/components/memory/memory_id_to_big/prover.rs line 233 at r1 (raw file):

        .collect_vec();

    let mut id_and_value_trace =

rename

Code quote:

let mut id_and_value_trace =

---

stwo_cairo_prover/crates/prover/src/components/memory/memory_id_to_big/prover.rs line 338 at r1 (raw file):

                }
            });
            let denom: PackedQM31 = lookup_elements.combine(&id_and_value);

WDYT?

Suggestion:

            let id = ackedM31::from_simd_unchecked(
                            (SIMD_ENUMERATION_0 + Simd::splat((vec_row * N_LANES) as u32))
                                | large_memory_value_id_tag,
                        );
            let denom: PackedQM31 = lookup_elements.combine(&chain!([id], self.big_values));

---

stwo_cairo_prover/crates/prover/src/components/memory/memory_id_to_big/component.rs line 90 at r1 (raw file):

            E::EF::from(-multiplicity),
            &chain!(
                [seq + E::F::from(M31::from(LARGE_MEMORY_VALUE_ID_TAG))],

rename

Suggestion:

[seq + E::F::from(M31::from(LARGE_MEMORY_VALUE_ID_BASE))],

---

stwo_cairo_prover/crates/prover/src/components/memory/memory_id_to_big/component.rs line 149 at r1 (raw file):

            &self.lookup_elements,
            E::EF::from(-multiplicity),
            &chain!([seq], value).collect_vec(),

consider to also add a base for small values where it equals 0.

Code quote:

&chain!([seq], value).collect_vec(),

---

stwo_cairo_prover/crates/prover/src/components/memory/memory_id_to_big/component.rs line 168 at r1 (raw file):

            self.big_log_size,
            self.small_log_size,
            self.small_log_size,

revert, I think we only have seq in the preprocessed trace

Also, do we want to remove this at all?

Code quote:

            self.big_log_size,
            self.big_log_size,
            self.small_log_size,
            self.small_log_size,

[ohad-starkware]

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @alon-f and @shaharsamocha7)

---

stwo_cairo_prover/crates/prover/src/components/memory/memory_id_to_big/component.rs line 90 at r1 (raw file):

Previously, shaharsamocha7 wrote…

rename

Done.

---

stwo_cairo_prover/crates/prover/src/components/memory/memory_id_to_big/component.rs line 149 at r1 (raw file):

Previously, shaharsamocha7 wrote…

consider to also add a base for small values where it equals 0.

this would also needs to change in the adapter, maybe at a later refactor?

---

stwo_cairo_prover/crates/prover/src/components/memory/memory_id_to_big/component.rs line 168 at r1 (raw file):

Previously, shaharsamocha7 wrote…

revert, I think we only have seq in the preprocessed trace

Also, do we want to remove this at all?

yes

---

stwo_cairo_prover/crates/prover/src/components/memory/memory_id_to_big/prover.rs line 233 at r1 (raw file):

Previously, shaharsamocha7 wrote…

rename

Done.

---

stwo_cairo_prover/crates/prover/src/components/memory/memory_id_to_big/prover.rs line 338 at r1 (raw file):

Previously, shaharsamocha7 wrote…

WDYT?

it doesnt compile
combine want &[T], so you would have to collect, which sucks
also self.big_values is by columns, so you don't have a row iterator to use in combine

[alon-f]

Reviewable status: 0 of 3 files reviewed, 6 unresolved discussions (waiting on @ohad-starkware and @shaharsamocha7)

---

stwo_cairo_prover/crates/prover/src/components/memory/memory_id_to_big/component.rs line 72 at r2 (raw file):

    fn evaluate<E: EvalAtRow>(&self, mut eval: E) -> E {
        let seq = eval.get_preprocessed_column(Seq::new(self.log_size()).id());

let seq_plus_base = ...

[alon-f]

Reviewable status: 0 of 3 files reviewed, 7 unresolved discussions (waiting on @ohad-starkware and @shaharsamocha7)

---

stwo_cairo_prover/crates/prover/src/components/memory/memory_id_to_big/component.rs line 132 at r2 (raw file):

    fn evaluate<E: EvalAtRow>(&self, mut eval: E) -> E {
        let seq = eval.get_preprocessed_column(Seq::new(self.log_size()).id());

let id = ...;
or let id = seq;

[alon-f]

Reviewable status: 0 of 3 files reviewed, 8 unresolved discussions (waiting on @ohad-starkware and @shaharsamocha7)

---

stwo_cairo_prover/crates/prover/src/components/memory/memory_id_to_big/prover.rs line 234 at r2 (raw file):

    let mut values_trace =
        std::iter::repeat_with(|| unsafe { BaseColumn::uninitialized(column_length) })

Do you like this?
How about something like

    let mut id_and_value_trace =
        (0..BIG_N_ID_AND_VALUE_COLUMNS).map(|_| unsafe {BaseColumn::uninitialized(column_length)}).collect_vec();

[ohad-starkware]

Reviewable status: 0 of 3 files reviewed, 6 unresolved discussions (waiting on @alon-f, @ohad-starkware, and @shaharsamocha7)

---

stwo_cairo_prover/crates/prover/src/components/memory/memory_id_to_big/component.rs line 132 at r2 (raw file):

Previously, alon-f wrote…

let id = ...;
or let id = seq;

wdyt?

---

stwo_cairo_prover/crates/prover/src/components/memory/memory_id_to_big/prover.rs line 234 at r2 (raw file):

Previously, alon-f wrote…

Do you like this?
How about something like

    let mut id_and_value_trace =
        (0..BIG_N_ID_AND_VALUE_COLUMNS).map(|_| unsafe {BaseColumn::uninitialized(column_length)}).collect_vec();

I think both are not ideal

[shaharsamocha7]

Reviewed 2 of 3 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @alon-f)