lambdaclass / cairo-vm

cairo-vm is a Rust implementation of the Cairo VM. Cairo (CPU Algebraic Intermediate Representation) is a programming language for writing provable programs, where one party can prove to another that a certain computation was executed correctly without the need for this party to re-execute the same program.
https://lambdaclass.github.io/cairo-vm
Apache License 2.0
504 stars 138 forks source link

Fix output serialization for cairo 1 #1645

Closed fmoletta closed 6 months ago

fmoletta commented 6 months ago

This PR reverts #1630 and re-implements it

codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 99.08257% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 97.68%. Comparing base (3ecc47e) to head (c0a5607).

Files Patch % Lines
cairo1-run/src/main.rs 99.08% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1645 +/- ## ========================================== + Coverage 97.64% 97.68% +0.03% ========================================== Files 92 91 -1 Lines 37656 37654 -2 ========================================== + Hits 36769 36782 +13 + Misses 887 872 -15 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

github-actions[bot] commented 6 months ago

Benchmark Results for unmodified programs :rocket:

Command Mean [s] Min [s] Max [s] Relative
base big_factorial 2.270 ± 0.023 2.243 2.309 1.00
head big_factorial 2.280 ± 0.011 2.258 2.288 1.00 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base big_fibonacci 2.258 ± 0.028 2.226 2.325 1.01 ± 0.01
head big_fibonacci 2.244 ± 0.017 2.212 2.263 1.00
Command Mean [s] Min [s] Max [s] Relative
base blake2s_integration_benchmark 8.448 ± 0.027 8.415 8.498 1.00
head blake2s_integration_benchmark 8.518 ± 0.058 8.449 8.603 1.01 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base compare_arrays_200000 2.345 ± 0.014 2.322 2.365 1.00 ± 0.01
head compare_arrays_200000 2.343 ± 0.010 2.322 2.358 1.00
Command Mean [s] Min [s] Max [s] Relative
base dict_integration_benchmark 1.463 ± 0.016 1.437 1.491 1.01 ± 0.01
head dict_integration_benchmark 1.453 ± 0.012 1.434 1.468 1.00
Command Mean [s] Min [s] Max [s] Relative
base field_arithmetic_get_square_benchmark 1.305 ± 0.006 1.296 1.316 1.00
head field_arithmetic_get_square_benchmark 1.314 ± 0.015 1.299 1.348 1.01 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base integration_builtins 8.507 ± 0.081 8.451 8.727 1.00
head integration_builtins 8.524 ± 0.090 8.449 8.719 1.00 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base keccak_integration_benchmark 8.718 ± 0.048 8.680 8.847 1.00
head keccak_integration_benchmark 8.807 ± 0.170 8.720 9.272 1.01 ± 0.02
Command Mean [s] Min [s] Max [s] Relative
base linear_search 2.349 ± 0.014 2.319 2.373 1.01 ± 0.01
head linear_search 2.334 ± 0.009 2.319 2.348 1.00
Command Mean [s] Min [s] Max [s] Relative
base math_cmp_and_pow_integration_benchmark 1.573 ± 0.009 1.559 1.583 1.00
head math_cmp_and_pow_integration_benchmark 1.581 ± 0.012 1.568 1.608 1.00 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base math_integration_benchmark 1.429 ± 0.012 1.417 1.448 1.00
head math_integration_benchmark 1.439 ± 0.009 1.424 1.448 1.01 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base memory_integration_benchmark 1.269 ± 0.006 1.260 1.277 1.01 ± 0.01
head memory_integration_benchmark 1.263 ± 0.006 1.254 1.272 1.00
Command Mean [s] Min [s] Max [s] Relative
base operations_with_data_structures_benchmarks 1.606 ± 0.007 1.595 1.615 1.00
head operations_with_data_structures_benchmarks 1.613 ± 0.017 1.596 1.648 1.00 ± 0.01
Command Mean [ms] Min [ms] Max [ms] Relative
base pedersen 589.9 ± 3.6 585.7 598.7 1.00
head pedersen 590.3 ± 3.2 587.0 597.1 1.00 ± 0.01
Command Mean [ms] Min [ms] Max [ms] Relative
base poseidon_integration_benchmark 967.9 ± 4.2 964.1 978.4 1.00
head poseidon_integration_benchmark 969.7 ± 9.5 959.5 985.2 1.00 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base secp_integration_benchmark 1.869 ± 0.013 1.854 1.898 1.00
head secp_integration_benchmark 1.878 ± 0.009 1.869 1.897 1.00 ± 0.01
Command Mean [ms] Min [ms] Max [ms] Relative
base set_integration_benchmark 700.4 ± 9.2 692.1 724.1 1.01 ± 0.01
head set_integration_benchmark 696.6 ± 3.7 692.5 703.7 1.00
Command Mean [s] Min [s] Max [s] Relative
base uint256_integration_benchmark 4.675 ± 0.060 4.625 4.803 1.00 ± 0.01
head uint256_integration_benchmark 4.660 ± 0.020 4.631 4.697 1.00
raphaelDkhn commented 6 months ago

Thanks @juanbono and @fmoletta this PR as well as #1630 successfully fix the issue with Nullable Felt252Dict printing. However, a problem arises when Nullable Felt252Dict types are embeded within a struct. The printing does not behave as expected. Let's take the following example:

struct NullableVec<T> {
    items: Felt252Dict<Nullable<T>>,
    len: usize,
}

fn main() -> NullableVec<u32> {
    let mut d: Felt252Dict<Nullable<u32>> = Default::default();

    // Populate the dictionary
    d.insert(0, nullable_from_box(BoxTrait::new(10)));
    d.insert(1, nullable_from_box(BoxTrait::new(20)));
    d.insert(2, nullable_from_box(BoxTrait::new(30)));

    // Return NullableVec
    NullableVec {
        items: d,
        len: 3,
    }
}

When executing the code with cargo run programs/null_vec.sierra --layout all_cairo --print_output, the expected output should display the values as follow{0:10 1:20 2:30} 3. However, the actual output is {0:8:0 1:8:1 2:8:2} 3.

fmoletta commented 6 months ago

Thanks @juanbono and @fmoletta this PR as well as #1630 successfully fix the issue with Nullable Felt252Dict printing. However, a problem arises when Nullable Felt252Dict types are embeded within a struct. The printing does not behave as expected. Let's take the following example:

struct NullableVec<T> {
    items: Felt252Dict<Nullable<T>>,
    len: usize,
}

fn main() -> NullableVec<u32> {
    let mut d: Felt252Dict<Nullable<u32>> = Default::default();

    // Populate the dictionary
    d.insert(0, nullable_from_box(BoxTrait::new(10)));
    d.insert(1, nullable_from_box(BoxTrait::new(20)));
    d.insert(2, nullable_from_box(BoxTrait::new(30)));

    // Return NullableVec
    NullableVec {
        items: d,
        len: 3,
    }
}

When executing the code with cargo run programs/null_vec.sierra --layout all_cairo --print_output, the expected output should display the values as follow{0:10 1:20 2:30} 3. However, the actual output is {0:8:0 1:8:1 2:8:2} 3.

Hello! Thanks for pointing this out! I recently pushed some changes to dereference referenced values upon deserialization which should fix this issue