feat: document trace layout constants using insta snapshots #2351
Conversation
| - Eval operation index: 5 | ||
| - ID 0 index: 6 | ||
| - ID 1 index: 9 | ||
| - ID 2 index: 12 |
There was a problem hiding this comment.
It would be a bit clearer if we ordered the columns by index, to show the overlap depending on the current block.
| Other Constants: | ||
| Number of columns: 16 | ||
| ACE init label: 8 | ||
| Instruction ID1 offset: 1073741824 |
There was a problem hiding this comment.
This is just a power of 2, not sure it's necessary.
air/src/trace/snapshots/miden_air__trace__tests__all_chiplet_column_ranges.snap
Outdated
Show resolved
Hide resolved
| Bitwise Chiplet: | ||
| Trace offset: 53 | ||
| Selector index: 53 | ||
| Input A range: 56..60 |
There was a problem hiding this comment.
Why are we reporting this here as well as in the Bitwise trace snapshot? We should either put all the info here, or in the individual chiplet snapshots. The same applies to the other chiplets.
| Column Indices: | ||
| - In span column: 16 | ||
| - Group count column: 17 | ||
| - Operation index column: 18 |
There was a problem hiding this comment.
Order these by index, and maybe format with the index before the label
| Capacity length: 4 | ||
| Capacity column range: 55..59 | ||
| Capacity domain index: 1 | ||
| Note: The capacity portion contains the first 4 elements of the |
There was a problem hiding this comment.
This comment doesn't seem necessary
|
|
||
| Other Constants: | ||
| Trace width: 5 | ||
| Kernel procedure call label: 16 |
There was a problem hiding this comment.
Format in binary in the same way it's defined by the constant
| Trace offset: 54 | ||
|
|
||
| Column Indices: | ||
| - Is read column: 54 |
| Value column range: 61..65 | ||
|
|
||
| Other Constants: | ||
| Trace width: 15 |
|
|
||
| Value Columns: |
There was a problem hiding this comment.
Value are columns so remove the separation
|
@hawkadrian, @adr1anh - is this PR still relevant? |
yes it is, will fix conflicts soon |
|
@hawkadrian once you're done with your rebase, please comment on #2495 so we can assign you |
…olumn_ranges.snap Co-authored-by: Adrian Hamelink <[email protected]>
hawkadrian
force-pushed
the
feat/document-trace-constants-with-insta
branch
from
b9eed2f to
1f0bd35
Compare
|
I did rebase. In theory everything should work like a clockwok now |
|
Thanks for rebasing. It seems like the ci tests are still failing. We use |
4 participants
Describe your changes
Adds snapshot tests using
instato document computed values of trace layout constants.Resolves #2221