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

chore: clarify to_radix docs examples #7230

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

iAmMichaelConnor
Copy link
Collaborator

Description

The current example always forces my brain to think, because the choice of converting 2 to base 256 doesn't actually demonstrate what to_radix does, because 2 base 256 is just 2.

I'm suggesting using a number larger than the base instead (259), so that we can see that it results in [3, 1] and [1, 3] for le and be cases resp.

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@iAmMichaelConnor iAmMichaelConnor changed the title docs: clarify to_radix docs examples doc: clarify to_radix docs examples Jan 29, 2025
@iAmMichaelConnor iAmMichaelConnor changed the title doc: clarify to_radix docs examples chore: clarify to_radix docs examples Jan 29, 2025
Copy link
Contributor

Changes to Brillig bytecode sizes

Generated at commit: 040f50f15dc87dccb18f471d0051dcdf070086bc, compared to commit: b602c23e3f96878124e711ccfd8108c0fc1fa827

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
fold_basic_nested_call_inliner_min -12 ✅ -19.35%
binary_operator_overloading_inliner_min -90 ✅ -20.09%
binary_operator_overloading_inliner_zero -90 ✅ -28.30%
fold_basic_inliner_min -24 ✅ -38.71%

Full diff report 👇
Program Brillig opcodes (+/-) %
poseidon2_inliner_min 378 (+1) +0.27%
regression_5252_inliner_min 3,571 (+1) +0.03%
global_var_regression_entry_points_inliner_min 133,784 (-80) -0.06%
global_var_regression_entry_points_inliner_zero 133,757 (-87) -0.07%
hashmap_inliner_min 9,791 (-13) -0.13%
uhashmap_inliner_min 8,330 (-26) -0.31%
slices_inliner_zero 2,084 (-9) -0.43%
sha256_var_padding_regression_inliner_zero 2,928 (-19) -0.64%
sha256_var_padding_regression_inliner_min 3,216 (-22) -0.68%
hashmap_inliner_zero 7,835 (-54) -0.68%
conditional_regression_short_circuit_inliner_min 1,117 (-8) -0.71%
6_inliner_min 1,016 (-8) -0.78%
conditional_regression_short_circuit_inliner_zero 917 (-8) -0.86%
higher_order_functions_inliner_min 1,337 (-12) -0.89%
6_inliner_zero 835 (-8) -0.95%
sha256_inliner_min 1,395 (-19) -1.34%
u128_inliner_min 2,263 (-32) -1.39%
sha256_inliner_zero 1,234 (-19) -1.52%
u128_inliner_zero 1,957 (-32) -1.61%
7_function_inliner_zero 518 (-14) -2.63%
aes128_encrypt_inliner_zero 388 (-11) -2.76%
6_array_inliner_min 446 (-16) -3.46%
6_array_inliner_zero 436 (-16) -3.54%
brillig_nested_arrays_inliner_zero 241 (-19) -7.31%
u16_support_inliner_min 59 (-6) -9.23%
u16_support_inliner_zero 59 (-6) -9.23%
brillig_cow_inliner_zero 239 (-33) -12.13%
fold_basic_nested_call_inliner_min 50 (-12) -19.35%
binary_operator_overloading_inliner_min 358 (-90) -20.09%
binary_operator_overloading_inliner_zero 228 (-90) -28.30%
fold_basic_inliner_min 38 (-24) -38.71%

Copy link
Contributor

Changes to number of Brillig opcodes executed

Generated at commit: 040f50f15dc87dccb18f471d0051dcdf070086bc, compared to commit: b602c23e3f96878124e711ccfd8108c0fc1fa827

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
conditional_regression_short_circuit_inliner_min -3,793 ✅ -43.32%
6_inliner_min -3,793 ✅ -43.85%
binary_operator_overloading_inliner_zero -236 ✅ -50.64%
fold_basic_inliner_min -40 ✅ -51.28%

Full diff report 👇
Program Brillig opcodes (+/-) %
poseidon2_inliner_min 873 (+1) +0.11%
regression_5252_inliner_min 1,031,270 (+2) +0.00%
uhashmap_inliner_min 196,981 (-180) -0.09%
hashmap_inliner_min 90,362 (-179) -0.20%
global_var_regression_entry_points_inliner_min 135,432 (-288) -0.21%
global_var_regression_entry_points_inliner_zero 135,311 (-336) -0.25%
higher_order_functions_inliner_min 2,273 (-18) -0.79%
hashmap_inliner_zero 74,602 (-626) -0.83%
7_function_inliner_zero 2,439 (-35) -1.41%
slices_inliner_zero 3,594 (-70) -1.91%
u128_inliner_min 48,483 (-3,734) -7.15%
u128_inliner_zero 40,944 (-3,510) -7.90%
aes128_encrypt_inliner_zero 3,075 (-364) -10.58%
6_array_inliner_min 2,540 (-350) -12.11%
6_array_inliner_zero 2,470 (-350) -12.41%
sha256_var_padding_regression_inliner_min 231,918 (-35,338) -13.22%
sha256_var_padding_regression_inliner_zero 230,407 (-35,186) -13.25%
sha256_inliner_min 12,303 (-4,253) -25.69%
sha256_inliner_zero 11,358 (-3,938) -25.75%
brillig_nested_arrays_inliner_zero 301 (-145) -32.51%
brillig_cow_inliner_zero 803 (-399) -33.19%
binary_operator_overloading_inliner_min 402 (-236) -36.99%
u16_support_inliner_min 51 (-31) -37.80%
u16_support_inliner_zero 51 (-31) -37.80%
fold_basic_nested_call_inliner_min 54 (-36) -40.00%
conditional_regression_short_circuit_inliner_zero 4,520 (-3,392) -42.87%
6_inliner_zero 4,440 (-3,392) -43.31%
conditional_regression_short_circuit_inliner_min 4,962 (-3,793) -43.32%
6_inliner_min 4,856 (-3,793) -43.85%
binary_operator_overloading_inliner_zero 230 (-236) -50.64%
fold_basic_inliner_min 38 (-40) -51.28%

Copy link
Contributor

Changes to circuit sizes

Generated at commit: 040f50f15dc87dccb18f471d0051dcdf070086bc, compared to commit: b602c23e3f96878124e711ccfd8108c0fc1fa827

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
conditional_regression_short_circuit -127 ✅ -34.79% -4,031 ✅ -36.19%
6 -127 ✅ -36.71% -4,030 ✅ -36.25%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
pedersen_check 15 (-4) -21.05% 3,170 (-2) -0.06%
global_var_regression_simple 66 (-61) -48.03% 2,914 (-152) -4.96%
fold_call_witness_condition 4 (-1) -20.00% 18 (-1) -5.26%
fold_basic 1 (-4) -80.00% 16 (-1) -5.88%
fold_basic_nested_call 1 (-2) -66.67% 16 (-1) -5.88%
sha256_var_padding_regression 3,468 (-350) -9.17% 162,236 (-31,417) -16.22%
sha256 1,279 (-157) -10.93% 20,564 (-4,019) -16.35%
global_var_regression_entry_points 6 (-20) -76.92% 18 (-10) -35.71%
conditional_regression_short_circuit 238 (-127) -34.79% 7,106 (-4,031) -36.19%
6 219 (-127) -36.71% 7,088 (-4,030) -36.25%

@TomAFrench TomAFrench enabled auto-merge January 29, 2025 19:39
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 1efdd58 Previous: 9f21824 Ratio
noir-lang_mimc_ 1 s 0 s +∞

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@TomAFrench TomAFrench added this pull request to the merge queue Jan 29, 2025
Merged via the queue into master with commit 4d37fb0 Jan 29, 2025
102 of 104 checks passed
@TomAFrench TomAFrench deleted the mc/clarify-to-radix-docs-examples branch January 29, 2025 19:58
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Jan 31, 2025
chore: bump gates diff (noir-lang/noir#7245)
feat: simplify subtraction from self to return zero (noir-lang/noir#7189)
feat: allow specifying multiple patterns in nargo test (noir-lang/noir#7186)
fix: Avoid type error when calling something with a type alias of a function (noir-lang/noir#7239)
feat: Allow resolved types in constructors (noir-lang/noir#7223)
chore: clarify to_radix docs examples (noir-lang/noir#7230)
chore: fix struct example (noir-lang/noir#7198)
feat(optimization): Add purity analysis to SSA (noir-lang/noir#7197)
chore: start tracking time to run critical library tests (noir-lang/noir#7221)
chore: Rework defunctionalize pass to not rely on DFG bugs (noir-lang/noir#7222)
fix(brillig): Globals entry point reachability analysis  (noir-lang/noir#7188)
chore: update docs to use devcontainer feature (noir-lang/noir#7206)
chore(ci): Add test for global vars entry points regression (noir-lang/noir#7209)
chore(ssa): Flip the SSA Brillig constraint check to off by default (noir-lang/noir#7211)
chore(docs): moving references to noir-starter to awesome-noir (noir-lang/noir#7203)
chore: build docs in the merge queue (noir-lang/noir#7218)
fix: correct reversed callstacks (noir-lang/noir#7212)
chore: exclude dependency fetching time from benchmarks (noir-lang/noir#7210)
feat(experimental): Support enums in comptime code (noir-lang/noir#7194)
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.

2 participants