Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add air_public_input flag to cairo1-run #1539

Merged
merged 21 commits into from
Jan 15, 2024
Merged

Conversation

fmoletta
Copy link
Contributor

@fmoletta fmoletta commented Jan 4, 2024

Air Public Input can now be obtained from cairo 1 proof_mode runs

@fmoletta fmoletta changed the base branch from main to cairo1-run-proof-arg January 4, 2024 16:04
@fmoletta fmoletta changed the title [WIP] Add air_public_input flag to cairo1-run Add air_public_input flag to cairo1-run Jan 4, 2024
Copy link

codecov bot commented Jan 4, 2024

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (352e8a8) 97.21% compared to head (5c19b3b) 97.22%.

Files Patch % Lines
cairo1-run/src/main.rs 87.23% 6 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1539   +/-   ##
=======================================
  Coverage   97.21%   97.22%           
=======================================
  Files          91       91           
  Lines       36990    37048   +58     
=======================================
+ Hits        35961    36020   +59     
+ Misses       1029     1028    -1     

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

Copy link

github-actions bot commented Jan 4, 2024

Benchmark Results for unmodified programs 🚀

Command Mean [s] Min [s] Max [s] Relative
base big_factorial 2.269 ± 0.022 2.243 2.319 1.00
head big_factorial 2.270 ± 0.028 2.243 2.325 1.00 ± 0.02
Command Mean [s] Min [s] Max [s] Relative
base big_fibonacci 2.241 ± 0.018 2.211 2.274 1.00 ± 0.01
head big_fibonacci 2.237 ± 0.012 2.225 2.267 1.00
Command Mean [s] Min [s] Max [s] Relative
base blake2s_integration_benchmark 8.281 ± 0.076 8.181 8.409 1.01 ± 0.01
head blake2s_integration_benchmark 8.215 ± 0.072 8.129 8.319 1.00
Command Mean [s] Min [s] Max [s] Relative
base compare_arrays_200000 2.285 ± 0.008 2.271 2.300 1.00
head compare_arrays_200000 2.306 ± 0.016 2.279 2.325 1.01 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base dict_integration_benchmark 1.461 ± 0.010 1.451 1.482 1.01 ± 0.01
head dict_integration_benchmark 1.449 ± 0.008 1.434 1.461 1.00
Command Mean [s] Min [s] Max [s] Relative
base field_arithmetic_get_square_benchmark 1.296 ± 0.010 1.286 1.320 1.00 ± 0.01
head field_arithmetic_get_square_benchmark 1.296 ± 0.011 1.281 1.312 1.00
Command Mean [s] Min [s] Max [s] Relative
base integration_builtins 8.327 ± 0.084 8.204 8.428 1.02 ± 0.01
head integration_builtins 8.201 ± 0.058 8.144 8.331 1.00
Command Mean [s] Min [s] Max [s] Relative
base keccak_integration_benchmark 8.531 ± 0.073 8.456 8.697 1.00 ± 0.01
head keccak_integration_benchmark 8.502 ± 0.083 8.412 8.650 1.00
Command Mean [s] Min [s] Max [s] Relative
base linear_search 2.352 ± 0.025 2.319 2.386 1.00
head linear_search 2.363 ± 0.013 2.350 2.394 1.00 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base math_cmp_and_pow_integration_benchmark 1.632 ± 0.010 1.620 1.645 1.01 ± 0.01
head math_cmp_and_pow_integration_benchmark 1.621 ± 0.017 1.605 1.647 1.00
Command Mean [s] Min [s] Max [s] Relative
base math_integration_benchmark 1.475 ± 0.008 1.464 1.488 1.01 ± 0.01
head math_integration_benchmark 1.462 ± 0.006 1.456 1.471 1.00
Command Mean [s] Min [s] Max [s] Relative
base memory_integration_benchmark 1.275 ± 0.009 1.262 1.288 1.00
head memory_integration_benchmark 1.276 ± 0.012 1.266 1.306 1.00 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base operations_with_data_structures_benchmarks 1.645 ± 0.008 1.634 1.658 1.01 ± 0.01
head operations_with_data_structures_benchmarks 1.629 ± 0.012 1.616 1.654 1.00
Command Mean [ms] Min [ms] Max [ms] Relative
base pedersen 607.9 ± 3.1 604.8 614.6 1.00
head pedersen 608.8 ± 3.9 604.8 617.4 1.00 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base poseidon_integration_benchmark 1.079 ± 0.010 1.070 1.104 1.00
head poseidon_integration_benchmark 1.082 ± 0.003 1.076 1.086 1.00 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base secp_integration_benchmark 1.958 ± 0.010 1.945 1.976 1.01 ± 0.01
head secp_integration_benchmark 1.930 ± 0.007 1.920 1.944 1.00
Command Mean [ms] Min [ms] Max [ms] Relative
base set_integration_benchmark 779.9 ± 1.7 778.0 783.3 1.00 ± 0.01
head set_integration_benchmark 779.3 ± 4.6 774.2 789.7 1.00
Command Mean [s] Min [s] Max [s] Relative
base uint256_integration_benchmark 4.634 ± 0.038 4.587 4.689 1.02 ± 0.01
head uint256_integration_benchmark 4.558 ± 0.028 4.526 4.629 1.00

@fmoletta fmoletta linked an issue Jan 8, 2024 that may be closed by this pull request
@fmoletta fmoletta marked this pull request as ready for review January 8, 2024 20:22
Comment on lines 402 to 410
for builtin in vm.builtin_runners.iter_mut() {
builtin.final_stack(
&vm.segments,
builtin_name_to_stack_pointer
.get(builtin.name())
.cloned()
.unwrap_or_default(),
)?;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't finalize_segments do this so we avoid exposing VirtualMachine::builtin_runners and VirtualMachine::segments?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In cairo 0, the builtin's stop pointers can be found contiguously at the end of the execution segment. Therefore we can take the current value of ap as stack_pointer and then iterate over the builtin runners calling final_stack (which will fetch the builtin's stop_ptr from memory[stack_ptr - 1]) on each of them, while advancing the stack_ptr by one each iteration to set all builtin's stop_ptr to the correct value.
In cairo 1 this doesn't happen, the builtin's stop_ptr are mixed up with the other return values so we can't iterate contiguously over a memory range to fetch the stop pointers. The logic above this code calculates an individual stack_ptr for each builtin, therefore we need to call each builtin's final_stack method independently, without being able to rely on read_return_values or get_builtins_final_stack like we do with cairo 0 execution.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a new method in the runner can do it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is kind of an specific behaviour to have as part of the public api, but I think we could add it to the vm as a docs_hidden method

let end = runner.initialize(&mut vm)?;

additional_initialization(&mut vm, data_len)?;

// Run it until the end/ infinite loop in proof_mode
runner.run_until_pc(end, &mut vm, &mut hint_processor)?;

if args.proof_mode {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you remove this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous code called runner.run_until_next_power_of_2(&mut vm, &mut hint_processor) if proof_mode was activated. This same function is called in the end_run method if proof_mode is activated

juanbono
juanbono previously approved these changes Jan 15, 2024
Base automatically changed from cairo1-run-proof-arg to main January 15, 2024 16:18
@Oppen Oppen dismissed juanbono’s stale review January 15, 2024 16:18

The base branch was changed.

@Oppen Oppen enabled auto-merge January 15, 2024 18:33
@Oppen Oppen added this pull request to the merge queue Jan 15, 2024
Merged via the queue into main with commit 4568a40 Jan 15, 2024
49 of 50 checks passed
@Oppen Oppen deleted the cairo-1-run-air-pub-input-flag branch January 15, 2024 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Air Public Inputs
3 participants