Signed functions for felts and sqrt #1150

mfachal · 2023-05-09T15:36:36Z

TITLE

Description

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.

That feature is for compatibility with another `Arbitrary` trait

…-felts-sqrt-and-proptests

github-actions · 2023-05-12T20:02:33Z

Benchmark Results for unmodified programs 🚀

Command	Mean [s]	Min [s]	Max [s]	Relative
`base blake2s_integration_benchmark`	15.992 ± 0.104	15.850	16.148	1.00
`head blake2s_integration_benchmark`	16.154 ± 0.213	15.982	16.675	1.01 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base compare_arrays_200000`	5.283 ± 0.074	5.216	5.460	1.00
`head compare_arrays_200000`	5.329 ± 0.060	5.274	5.454	1.01 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base dict_integration_benchmark`	3.516 ± 0.067	3.448	3.647	1.00
`head dict_integration_benchmark`	3.544 ± 0.041	3.494	3.637	1.01 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base factorial_multirun`	5.596 ± 0.051	5.522	5.683	1.00
`head factorial_multirun`	5.663 ± 0.063	5.556	5.761	1.01 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base fibonacci_1000_multirun`	4.843 ± 0.085	4.695	4.965	1.00
`head fibonacci_1000_multirun`	4.880 ± 0.075	4.808	5.035	1.01 ± 0.02

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base field_arithmetic_get_square_benchmark`	217.5 ± 2.8	213.6	221.6	1.00
`head field_arithmetic_get_square_benchmark`	218.2 ± 7.3	210.8	230.4	1.00 ± 0.04

Command	Mean [s]	Min [s]	Max [s]	Relative
`base integration_builtins`	14.484 ± 0.154	14.324	14.803	1.00 ± 0.02
`head integration_builtins`	14.452 ± 0.181	14.120	14.796	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base keccak_integration_benchmark`	16.567 ± 0.814	16.177	18.861	1.02 ± 0.05
`head keccak_integration_benchmark`	16.295 ± 0.165	16.159	16.610	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base linear_search`	5.300 ± 0.118	5.185	5.599	1.00
`head linear_search`	5.367 ± 0.087	5.226	5.524	1.01 ± 0.03

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_cmp_and_pow_integration_benchmark`	3.699 ± 0.033	3.656	3.754	1.00
`head math_cmp_and_pow_integration_benchmark`	3.777 ± 0.040	3.682	3.820	1.02 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_integration_benchmark`	3.549 ± 0.054	3.459	3.633	1.00
`head math_integration_benchmark`	3.585 ± 0.038	3.517	3.653	1.01 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base memory_integration_benchmark`	3.006 ± 0.022	2.984	3.050	1.00
`head memory_integration_benchmark`	3.034 ± 0.011	3.012	3.049	1.01 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base operations_with_data_structures_benchmarks`	3.454 ± 0.060	3.368	3.541	1.01 ± 0.02
`head operations_with_data_structures_benchmarks`	3.434 ± 0.034	3.364	3.468	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base pedersen`	1.159 ± 0.018	1.139	1.202	1.00 ± 0.02
`head pedersen`	1.154 ± 0.022	1.130	1.203	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base poseidon_integration_benchmark`	2.057 ± 0.091	1.989	2.308	1.02 ± 0.05
`head poseidon_integration_benchmark`	2.025 ± 0.025	2.000	2.078	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base secp_integration_benchmark`	4.396 ± 0.042	4.333	4.471	1.00
`head secp_integration_benchmark`	4.422 ± 0.060	4.352	4.566	1.01 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base set_integration_benchmark`	2.704 ± 0.032	2.672	2.768	1.00
`head set_integration_benchmark`	2.729 ± 0.052	2.660	2.847	1.01 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base uint256_integration_benchmark`	9.964 ± 0.093	9.797	10.074	1.01 ± 0.01
`head uint256_integration_benchmark`	9.909 ± 0.051	9.802	9.955	1.00

felt/src/lib.rs

…aclass/cairo-rs into arbitrary-felts-sqrt-and-proptests

codecov · 2023-05-15T18:16:24Z

Codecov Report

Merging #1150 (d2bd6d0) into main (7f0517e) will increase coverage by 0.15%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1150      +/-   ##
==========================================
+ Coverage   97.57%   97.72%   +0.15%     
==========================================
  Files          89       89              
  Lines       35739    35651      -88     
==========================================
- Hits        34872    34840      -32     
+ Misses        867      811      -56

Impacted Files	Coverage Δ
src/math_utils.rs	`99.24% <ø> (-0.06%)`	⬇️
felt/src/bigint_felt.rs	`95.33% <100.00%> (+0.32%)`	⬆️
felt/src/lib.rs	`90.68% <100.00%> (+4.53%)`	⬆️
.../hint_processor/builtin_hint_processor/ec_utils.rs	`99.01% <100.00%> (ø)`
...cessor/builtin_hint_processor/find_element_hint.rs	`96.44% <100.00%> (-0.29%)`	⬇️
...int_processor/builtin_hint_processor/math_utils.rs	`98.06% <100.00%> (+<0.01%)`	⬆️
...processor/cairo_1_hint_processor/hint_processor.rs	`84.67% <100.00%> (ø)`

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

* Add arbitrary function * Update tests * Fix: remove arbitrary feature for BigInt That feature is for compatibility with another `Arbitrary` trait * Fix: construction tests shouldn't receive felt * Fix: lower part was capped by PH instead of PL * Impl `Arbitrary` for `FeltBigInt` and `Felt252` * WIP abs * WIP abs proptests * WIP: signed tests don't contradict implementation * signed functions and sqrt * Uncomment + fix sqrt impl * Fix sqrt_is_inv_square test * Remove sqrt implementation from FeltBigInt * Remove sqrt fn from src/math_utils.rs * Restore proptest after merge * Restore merge changes * Remove now useless line from find_element hint * Remove now useless lines from search_sorted_lower hint * Restore line, remove test * Update to_signed_felt * Fix is_positive hint implementation * Clippy * Add changelog entry * Remove pub * Remove redundant check * Restore fmt * Fix sqrt proptest * Fix SquareRoot hint --------- Co-authored-by: Tomás <[email protected]> Co-authored-by: Tomás <[email protected]> Co-authored-by: Federica <[email protected]> Co-authored-by: fmoletta <[email protected]> Co-authored-by: Pedro Fontana <[email protected]>

MegaRedHand and others added 23 commits April 28, 2023 18:35

Add arbitrary function

e3f91fd

Update tests

3361cc9

Fix: remove arbitrary feature for BigInt

b3c9e7f

That feature is for compatibility with another `Arbitrary` trait

Fix: construction tests shouldn't receive felt

259d938

Fix: lower part was capped by PH instead of PL

e8355e7

Impl Arbitrary for FeltBigInt and Felt252

d39d10e

Merge branch 'main' into arbitrary-felts

1570caa

Merge branch 'main' into arbitrary-felts

2c49b86

WIP abs

d626f14

WIP abs proptests

7f06acd

WIP: signed tests don't contradict implementation

d4909f0

signed functions and sqrt

391d522

Uncomment + fix sqrt impl

357b2cd

Fix sqrt_is_inv_square test

4c94d0b

Remove sqrt implementation from FeltBigInt

9af72f5

Remove sqrt fn from src/math_utils.rs

b7b5d81

Merge branch 'main' of github.com:lambdaclass/cairo-rs into arbitrary…

e0975c7

…-felts-sqrt-and-proptests

Restore proptest after merge

3cc0d77

Restore merge changes

4460c28

Remove now useless line from find_element hint

f9ad7a6

Remove now useless lines from search_sorted_lower hint

ff51e0b

Restore line, remove test

73070eb

Update to_signed_felt

32874f3

fmoletta self-assigned this May 12, 2023

fmoletta added 6 commits May 12, 2023 16:14

Fix is_positive hint implementation

0aa9999

Clippy

c6e8377

Add changelog entry

22b9a0f

Remove pub

a254201

Remove redundant check

6265530

Restore fmt

95877d1

Merge branch 'main' into arbitrary-felts-sqrt-and-proptests

a3f07cd

fmoletta marked this pull request as ready for review May 12, 2023 19:38

fmoletta requested review from igaray, Oppen, fmoletta, entropidelic, juanbono, pefontana and Juan-M-V as code owners May 12, 2023 19:38

Oppen reviewed May 13, 2023

View reviewed changes

felt/src/lib.rs Outdated Show resolved Hide resolved

fmoletta added 2 commits May 15, 2023 10:17

Fix sqrt proptest

1d8dd0b

Merge branch 'arbitrary-felts-sqrt-and-proptests' of github.com:lambd…

85ed0dc

…aclass/cairo-rs into arbitrary-felts-sqrt-and-proptests

Oppen approved these changes May 15, 2023

View reviewed changes

pefontana and others added 3 commits May 15, 2023 11:09

Merge branch 'main' into arbitrary-felts-sqrt-and-proptests

ab1ad00

Fix SquareRoot hint

ddfd395

Merge branch 'arbitrary-felts-sqrt-and-proptests' of github.com:lambd…

d2bd6d0

…aclass/cairo-rs into arbitrary-felts-sqrt-and-proptests

pefontana approved these changes May 15, 2023

View reviewed changes

pefontana enabled auto-merge May 15, 2023 18:12

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

Merged via the queue into main with commit 605b9d8 May 15, 2023

pefontana deleted the arbitrary-felts-sqrt-and-proptests branch May 15, 2023 18:40

pefontana linked an issue May 15, 2023 that may be closed by this pull request

Review proptest failures #1076

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Signed functions for felts and sqrt #1150

Signed functions for felts and sqrt #1150

mfachal commented May 9, 2023

github-actions bot commented May 12, 2023 •

edited

Loading

codecov bot commented May 15, 2023

Signed functions for felts and sqrt #1150

Signed functions for felts and sqrt #1150

Conversation

mfachal commented May 9, 2023

TITLE

Description

Checklist

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

codecov bot commented May 15, 2023

Codecov Report

github-actions bot commented May 12, 2023 •

edited

Loading