Memory is offseted by 1

[shaharsamocha7]

This change is 

[shaharsamocha7]

• Memory is offseted by 1 #363 👈 (View in Graphite)
• main

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

[DavidLevitGurevich]

Reviewed 1 of 3 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @ilyalesokhin-starkware and @shaharsamocha7)

---

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

impl ClaimGenerator {
    pub fn new(memory: &Memory) -> Self {
        let ids = (1..memory.address_to_id.len())

this is now not a power of two right?

---

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

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

I think the `+1 should be here

Code quote:

let address = eval.get_preprocessed_column(Seq::new(self.log_size()).id());

---

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

            // Addresses are offseted by 1, as 0 address is reserved.
            let address =
                address.clone() + E::F::from(M31((i * (1 << self.log_size()) + 1) as u32));

doesn't it add extra+1 per each iteration of the for loop?

[anatgstarkware]

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

---

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

    pub fn get_id(&self, input: BaseField) -> M31 {
        M31(self.ids[input.0 as usize - 1])

Don't you want us to change this on our side?

---

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

    pub fn add_m31(&self, addr: BaseField) {
        self.multiplicities.increase_at(addr.0 - 1);

Don't you want us to change this on our side?

[shaharsamocha7]

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @anatgstarkware, @DavidLevitGurevich, and @ilyalesokhin-starkware)

---

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

Previously, DavidLevitGurevich wrote…

I think the `+1 should be here

It could, but it's the same, right?

---

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

Previously, DavidLevitGurevich wrote…

doesn't it add extra+1 per each iteration of the for loop?

It is, we need it for every split

---

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

Previously, DavidLevitGurevich wrote…

this is now not a power of two right?

no but it wasn't a power of two even before
We pad it to pow2 in write_trace function

---

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

Previously, anatgstarkware (anatg) wrote…

Don't you want us to change this on our side?

I don't think so, it is an internal implementation of the memory, should we change all the other components for that?

---

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

Previously, anatgstarkware (anatg) wrote…

Don't you want us to change this on our side?

same

[alon-f]

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @anatgstarkware, @DavidLevitGurevich, @ilyalesokhin-starkware, and @shaharsamocha7)

---

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

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

seq_plus_one = ... +1

---

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

        // Set up data.
        let memory_addreses = [1, 2, 3, 4, 5, 6, 7, 8, 15, 16, 17, 18, 1, 1, 1, 1];
        let expected = memory_addreses

fix to addresses if you want in this pr

[DavidLevitGurevich]

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @anatgstarkware, @ilyalesokhin-starkware, and @shaharsamocha7)

---

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

Previously, shaharsamocha7 wrote…

It is, we need it for every split

I am probably missing something, you are overloading address so the +1 is cumulatively added again in every iteration, so in iteration i the offset is +i isn't it?

---

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

Previously, shaharsamocha7 wrote…

no but it wasn't a power of two even before
We pad it to pow2 in write_trace function

I just wonder how could the end of the range not change if you trimmed the start and the length did not change?

[ilyalesokhin-starkware]

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

Previously, DavidLevitGurevich wrote…

I just wonder how could the end of the range not change if you trimmed the start and the length did not change?

Why did this change?
why does the address range effect the number of ids?

[ilyalesokhin-starkware]

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

    }

    pub fn get_id(&self, input: BaseField) -> M31 {

Does this function return the id at a specific address?
input == address?

Code quote:

 pub fn get_id(&self, input: BaseField) -> M31 {

[ilyalesokhin-starkware]

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

    pub fn add_m31(&self, addr: BaseField) {
        self.multiplicities.increase_at(addr.0 - 1);

can you move the change inside increase_at?

Code quote:

    self.multiplicities.increase_at(addr.0 - 1);

[anatgstarkware]

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

Previously, shaharsamocha7 wrote…

I don't think so, it is an internal implementation of the memory, should we change all the other components for that?

You get the input from the infra, so it's not really internal. It means changing only one place in the infra.