CairoRunner.run_until_pc_with_steps_limit #1181

pefontana · 2023-05-26T15:28:20Z

Add CairoRunner.run_until_pc_with_steps_limit method

Description of the pull request changes and motivation.

Checklist

Linked to Github Issue
Unit tests added
Integration tests added.
This change requires new documentation.
- Documentation has been added/updated.
- CHANGELOG has been updated.

codecov · 2023-05-26T15:46:43Z

Codecov Report

Merging #1181 (3cbdbba) into main (90ecedd) will decrease coverage by 0.01%.
The diff coverage is 97.22%.

@@            Coverage Diff             @@
##             main    #1181      +/-   ##
==========================================
- Coverage   97.70%   97.70%   -0.01%     
==========================================
  Files          89       89              
  Lines       35739    35847     +108     
==========================================
+ Hits        34920    35025     +105     
- Misses        819      822       +3

Impacted Files	Coverage Δ
src/vm/runners/cairo_runner.rs	`98.09% <97.22%> (-0.03%)`	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

github-actions · 2023-05-26T16:00:04Z

Benchmark Results for unmodified programs 🚀

Command	Mean [s]	Min [s]	Max [s]	Relative
`base blake2s_integration_benchmark`	12.829 ± 0.069	12.725	12.933	1.01 ± 0.01
`head blake2s_integration_benchmark`	12.705 ± 0.024	12.663	12.733	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base compare_arrays_200000`	4.293 ± 0.054	4.241	4.433	1.00
`head compare_arrays_200000`	4.308 ± 0.075	4.256	4.497	1.00 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base dict_integration_benchmark`	2.845 ± 0.024	2.805	2.882	1.00 ± 0.01
`head dict_integration_benchmark`	2.844 ± 0.015	2.827	2.877	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base factorial_multirun`	4.507 ± 0.018	4.483	4.531	1.00
`head factorial_multirun`	4.514 ± 0.032	4.494	4.600	1.00 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base fibonacci_1000_multirun`	3.881 ± 0.022	3.862	3.928	1.00
`head fibonacci_1000_multirun`	3.901 ± 0.028	3.874	3.965	1.01 ± 0.01

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base field_arithmetic_get_square_benchmark`	184.5 ± 1.9	182.1	187.2	1.00 ± 0.01
`head field_arithmetic_get_square_benchmark`	183.9 ± 1.0	182.8	185.8	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base integration_builtins`	11.784 ± 0.153	11.617	12.025	1.01 ± 0.01
`head integration_builtins`	11.657 ± 0.062	11.576	11.779	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base keccak_integration_benchmark`	13.204 ± 0.186	12.951	13.503	1.02 ± 0.02
`head keccak_integration_benchmark`	12.934 ± 0.145	12.806	13.226	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base linear_search`	4.287 ± 0.034	4.256	4.366	1.00 ± 0.01
`head linear_search`	4.279 ± 0.029	4.251	4.341	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_cmp_and_pow_integration_benchmark`	3.100 ± 0.034	3.077	3.193	1.00 ± 0.01
`head math_cmp_and_pow_integration_benchmark`	3.098 ± 0.012	3.083	3.118	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_integration_benchmark`	2.867 ± 0.031	2.840	2.946	1.00 ± 0.01
`head math_integration_benchmark`	2.861 ± 0.008	2.848	2.872	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base memory_integration_benchmark`	2.437 ± 0.007	2.427	2.448	1.00
`head memory_integration_benchmark`	2.439 ± 0.011	2.425	2.462	1.00 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base operations_with_data_structures_benchmarks`	2.815 ± 0.014	2.799	2.839	1.00
`head operations_with_data_structures_benchmarks`	2.830 ± 0.024	2.805	2.872	1.01 ± 0.01

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base pedersen`	913.7 ± 5.1	909.7	926.0	1.00 ± 0.01
`head pedersen`	910.6 ± 2.4	906.5	914.5	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base poseidon_integration_benchmark`	1.625 ± 0.020	1.612	1.681	1.01 ± 0.01
`head poseidon_integration_benchmark`	1.616 ± 0.004	1.611	1.622	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base secp_integration_benchmark`	3.487 ± 0.010	3.471	3.507	1.00
`head secp_integration_benchmark`	3.500 ± 0.025	3.468	3.550	1.00 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base set_integration_benchmark`	1.939 ± 0.009	1.927	1.952	1.01 ± 0.01
`head set_integration_benchmark`	1.921 ± 0.010	1.911	1.943	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base uint256_integration_benchmark`	8.284 ± 0.023	8.255	8.341	1.00 ± 0.00
`head uint256_integration_benchmark`	8.262 ± 0.014	8.246	8.287	1.00

fmoletta

Aren't run_until_steps and run_for_steps already satisfying this functionality?

pefontana · 2023-05-29T14:00:13Z

Aren't run_until_steps and run_for_steps already satisfying this functionality?

They are similar but no

run_for_steps returns an Error if you finish executing the program and OK if the execution didn't finish
run_until_pc_with_steps_limit returns OK if you finish executing the Program, and Error if you don't finish the execution

juanbono · 2023-05-29T14:46:32Z

src/vm/errors/vm_errors.rs

@@ -93,6 +93,8 @@ pub enum VirtualMachineError {
    NoImm,
    #[error("Execution reached the end of the program. Requested remaining steps: {0}.")]
    EndOfProgram(usize),
+    #[error("Couldnt reached the end of the program. Executed steps: {0}.")]
+    StepsLimit(u64),


are we sure that u64 is enough?

Type: reached -> reach

Couldnt -> Could not (formal english please)

I think u64 will be enough, u64::MAX = 18_446_744_073_709_551_615u64
Also current_step and the other methods uses usize for the step counters

thanks Aki!
Done 3cbdbba

* Add CairoRunner.run_until_pc_with_steps_limit * Add unit test * Add integration test * Minor API change * Update CHANGELOG.md * Add doc * change error msg

pefontana added 4 commits May 26, 2023 08:23

Add CairoRunner.run_until_pc_with_steps_limit

c68c1ed

Add unit test

a815a21

Add integration test

8b9148d

Minor API change

076fc4e

pefontana requested review from igaray, Oppen, fmoletta, entropidelic, juanbono and Juan-M-V as code owners May 26, 2023 15:28

pefontana added 2 commits May 26, 2023 12:29

Update CHANGELOG.md

a5a4a35

Add doc

65041b9

fmoletta reviewed May 29, 2023

View reviewed changes

fmoletta approved these changes May 29, 2023

View reviewed changes

juanbono reviewed May 29, 2023

View reviewed changes

juanbono approved these changes May 29, 2023

View reviewed changes

change error msg

3cbdbba

pefontana enabled auto-merge May 29, 2023 15:13

This was referenced May 29, 2023

Implement run resources funcs #1175

Merged

Add RunResources structure #1173

Closed

pefontana added this pull request to the merge queue May 29, 2023

Merged via the queue into main with commit 45bf5c8 May 29, 2023
30 of 31 checks passed

pefontana deleted the CairoRunner-run_until_pc_with_step_limits branch May 29, 2023 16:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CairoRunner.run_until_pc_with_steps_limit #1181

CairoRunner.run_until_pc_with_steps_limit #1181

pefontana commented May 26, 2023

codecov bot commented May 26, 2023 •

edited

Loading

github-actions bot commented May 26, 2023 •

edited

Loading

fmoletta left a comment

pefontana commented May 29, 2023

juanbono May 29, 2023

igaray May 29, 2023

igaray May 29, 2023

pefontana May 29, 2023

pefontana May 29, 2023

CairoRunner.run_until_pc_with_steps_limit #1181

CairoRunner.run_until_pc_with_steps_limit #1181

Conversation

pefontana commented May 26, 2023

Add CairoRunner.run_until_pc_with_steps_limit method

Checklist

codecov bot commented May 26, 2023 • edited Loading

Codecov Report

github-actions bot commented May 26, 2023 • edited Loading

fmoletta left a comment

Choose a reason for hiding this comment

pefontana commented May 29, 2023

juanbono May 29, 2023

Choose a reason for hiding this comment

igaray May 29, 2023

Choose a reason for hiding this comment

igaray May 29, 2023

Choose a reason for hiding this comment

pefontana May 29, 2023

Choose a reason for hiding this comment

pefontana May 29, 2023

Choose a reason for hiding this comment

codecov bot commented May 26, 2023 •

edited

Loading

github-actions bot commented May 26, 2023 •

edited

Loading