search

0xPolygonMiden / miden-vm

STARK-based virtual machine
MIT License
630 stars 160 forks source link

Introduce a newtype for to represent a row in a trace #1236

Closed Al-Kindi-0 closed 3 months ago

Al-Kindi-0 commented 9 months ago

We should probably introduce a newtype to represent a row. Maybe we call it Cycle or Step. It could look something like this:

pub struct Step(u32);

And then we can implement both From<Step> for u32 and From<Step> for usize.

_Originally posted by @bobbinth in https://github.com/0xPolygonMiden/miden-vm/pull/1229#discussion_r1482721654_

sergerad commented 3 months ago

@bobbinth is this still desired? Will make a PR if so. LMK which crate the newtype should be declared in.

bobbinth commented 3 months ago

Yes, implementing this would be great! I'd probably put it either into the air carte or the core create (maybe air is slightly preferred).

sergerad commented 3 months ago

@bobbinth just checking whether Step should be limited to u32 or whether it should be usize (variably sized based on arch).

The issue and related comment suggest u32 with the example:

pub struct Step(u32);

This potentially has consequences for constants like OP_CYCLE_LEN which relate to rows conceptually.

bobbinth commented 3 months ago

Yes, limiting to u32 should be fine (the Goldilocks fields does not support bigger traces anyways, at least not without a lot of extra work). I would add a conversion From<Step> for usize though so that we could easily convert Step to usize values for indexing in vectors etc.

bobbinth commented 3 months ago

Closed by #1408. Thank you again @sergerad!