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.

Sign up for GitHub

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

Jump to bottom

Add ensure_no_std build to CI #1232

Merged
merged 25 commits into from
Jun 16, 2023
Merged

Add ensure_no_std build to CI #1232

merged 25 commits into from
Jun 16, 2023

Conversation

MegaRedHand
Copy link
Contributor

TITLE

Description

This PR builds the recently added ensure_no_std crate in the CI, to ensure that no-std works.

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.

@MegaRedHand MegaRedHand added the pipelines This PR/issue is exclusively about improving our CI label Jun 13, 2023
@github-actions
Copy link

github-actions bot commented Jun 13, 2023 •
edited

Benchmark Results for unmodified programs 🚀

Command Mean [s] Min [s] Max [s] Relative
base factorial_multirun 3.707 ± 0.121 3.617 4.030 1.02 ± 0.03
head factorial_multirun 3.638 ± 0.040 3.599 3.709 1.00
Command Mean [s] Min [s] Max [s] Relative
base fibonacci_1000_multirun 3.113 ± 0.048 3.066 3.186 1.01 ± 0.02
head fibonacci_1000_multirun 3.088 ± 0.022 3.060 3.122 1.00

@codecov
Copy link

codecov bot commented Jun 13, 2023 •
edited

Codecov Report

Merging #1232 (c08a4b8) into main (80dd475) will decrease coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1232      +/-   ##
==========================================
- Coverage   97.62%   97.57%   -0.05%     
==========================================
  Files          89       89              
  Lines       37001    36113     -888     
==========================================
- Hits        36122    35238     -884     
+ Misses        879      875       -4
Impacted Files Coverage Δ
vm/src/math_utils/is_prime.rs 100.00% <ø> (+0.44%) ⬆️
vm/src/math_utils/mod.rs 99.39% <ø> (ø)

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

@MegaRedHand MegaRedHand marked this pull request as ready for review June 13, 2023 21:19
@MegaRedHand MegaRedHand marked this pull request as draft June 13, 2023 21:58
Oppen
Oppen reviewed Jun 13, 2023
View reviewed changes
.github/workflows/rust.yml Outdated Show resolved Hide resolved
Oppen
Oppen reviewed Jun 13, 2023
View reviewed changes
ensure_no_std/Cargo.toml Outdated Show resolved Hide resolved
@d-roak d-roak changed the title Add ensure_no_std build to CI Fix no_std compatibility in vm and add ensure_no_std build to CI Jun 15, 2023
@d-roak
Copy link
Contributor

d-roak commented Jun 15, 2023

It seems we can revert some commits here in favor of #1238

@MegaRedHand MegaRedHand marked this pull request as ready for review June 15, 2023 21:43
@github-actions
Copy link

Benchmark Results for modified programs 🚀

Command Mean [s] Min [s] Max [s] Relative
head blake2s_integration_benchmark 12.740 ± 0.207 12.542 13.164 1.00
Command Mean [s] Min [s] Max [s] Relative
head compare_arrays_200000 4.550 ± 0.186 4.323 4.971 1.00
Command Mean [s] Min [s] Max [s] Relative
head dict_integration_benchmark 2.940 ± 0.025 2.894 2.981 1.00
Command Mean [s] Min [s] Max [s] Relative
head field_arithmetic_get_square_benchmark 2.211 ± 0.016 2.190 2.236 1.00
Command Mean [s] Min [s] Max [s] Relative
head integration_builtins 11.213 ± 0.168 10.970 11.450 1.00
Command Mean [s] Min [s] Max [s] Relative
head keccak_integration_benchmark 12.921 ± 0.277 12.558 13.449 1.00
Command Mean [s] Min [s] Max [s] Relative
head linear_search 4.384 ± 0.037 4.338 4.447 1.00
Command Mean [s] Min [s] Max [s] Relative
head math_cmp_and_pow_integration_benchmark 3.119 ± 0.025 3.091 3.162 1.00
Command Mean [s] Min [s] Max [s] Relative
head math_integration_benchmark 2.882 ± 0.027 2.853 2.950 1.00
Command Mean [s] Min [s] Max [s] Relative
head memory_integration_benchmark 2.514 ± 0.039 2.485 2.617 1.00
Command Mean [s] Min [s] Max [s] Relative
head operations_with_data_structures_benchmarks 2.842 ± 0.039 2.806 2.938 1.00
Command Mean [ms] Min [ms] Max [ms] Relative
head pedersen 985.5 ± 9.7 972.2 999.9 1.00
Command Mean [s] Min [s] Max [s] Relative
head poseidon_integration_benchmark 1.774 ± 0.025 1.746 1.833 1.00
Command Mean [s] Min [s] Max [s] Relative
head secp_integration_benchmark 3.421 ± 0.029 3.388 3.490 1.00
Command Mean [s] Min [s] Max [s] Relative
head set_integration_benchmark 1.890 ± 0.024 1.861 1.928 1.00
Command Mean [s] Min [s] Max [s] Relative
head uint256_integration_benchmark 7.937 ± 0.058 7.855 8.033 1.00

@MegaRedHand MegaRedHand changed the title Fix no_std compatibility in vm and add ensure_no_std build to CI Add ensure_no_std build to CI Jun 15, 2023
@MegaRedHand MegaRedHand added this pull request to the merge queue Jun 15, 2023
@MegaRedHand MegaRedHand mentioned this pull request Jun 15, 2023
Merged
Merged via the queue into main with commit 8a72ddc Jun 16, 2023
31 checks passed
@MegaRedHand MegaRedHand deleted the add-ensure_no_std-to-CI branch June 16, 2023 00:29
kariy pushed a commit to dojoengine/cairo-rs that referenced this pull request Jun 23, 2023
@MegaRedHand @kariy
Add ensure_no_std build to CI (lambdaclass#1232)
59d23c4 
* Add ensure_no_std build to CI

* Add print to test

* Install wasm32 target for no-std testing

* Fix: added option in wrong job

* Try another breaking change

* Remove "alloc" feature in Cargo.toml

* Also add compilation with the 'alloc' option

* Add cairo-vm to ensure_no_std deps

* Remove alloc feature

* Fix (WIP)

* comment vm (not no_std ready); use nightly toolchain

* add Timothée num-prime fork and fix no-std support

* run cargo fmt

* fix clippy; add original num_prime for tests; ignore unused FloatCore

* fix typo on unused_variables instead imports

* remove prime computations for non default primality test config

* Split implementation of is_prime into std/no_std

* Remove unused import

* Change some constants

* Update dir name

* Fix errors

* Use modpow, and increase time of bench

* Merge smoke and smoke-no_std

* Use mul instead of pow

---------

Co-authored-by: João Carvalho <joao.carvalho@tripleoak.pt>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Oppen Oppen Oppen approved these changes

@juanbono juanbono juanbono approved these changes

@fmoletta fmoletta fmoletta approved these changes

@igaray igaray Awaiting requested review from igaray igaray is a code owner

@entropidelic entropidelic Awaiting requested review from entropidelic

@pefontana pefontana Awaiting requested review from pefontana pefontana is a code owner

@Juan-M-V Juan-M-V Awaiting requested review from Juan-M-V

Assignees
No one assigned
Labels
pipelines This PR/issue is exclusively about improving our CI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@MegaRedHand @d-roak @Oppen @juanbono @fmoletta