Minor spelling and add deprecation notice to `LIMB_BITS`. #180

raldone01 · 2024-11-28T19:37:59Z

While reading through the source code I found some minor spelling mistakes.
Also I noticed the deprecation notice for BITS on the Integer trait in lexical-util.
So I added it to bigint aswell.
Also the LIMB_BITS is no longer duplicated under a cfg.

mem::size_of is const since 1.25 so it should be fine?

I just noticed that the readme says: rustc 1.63+ is supported.
If you want I can just apply the deprecation notices for ::BITS.

I have provided two commits.
The first one just adds the deprecation notice and the second one does the deprecation.

Off topic question:

Do you happen to know any way to tell whether a string to float conversion was exact or not?
Converting to a string to float and back to a string again to check for equality seems like a bad idea.

raldone01 · 2024-12-02T16:02:22Z

Hello,
I see some runs have failed.
Any way I can locally build with all required feature combinations?
Something like the check in rustc?

Ah I just found the development guide.
I will fix the issues soon.
Sorry.

Alexhuszagh · 2024-12-02T16:06:42Z

This is excellent, thanks for the careful overview of the code. A few of the unit tests need to be patched and this will be good to go. I'll post the diff now.

Alexhuszagh · 2024-12-02T16:08:24Z

@raldone01 This should be sufficient:

diff --git a/lexical-benchmark/algorithm/bigint.rs b/lexical-benchmark/algorithm/bigint.rs
index 9d20944..31394f9 100644
--- a/lexical-benchmark/algorithm/bigint.rs
+++ b/lexical-benchmark/algorithm/bigint.rs
@@ -15,7 +15,7 @@ fn standard_pow(big: &mut bigint::Bigint, exp: u32) {
 fn small_pow(big: &mut bigint::Bigint, mut exp: u32) {
     let shift = exp as usize;
     // Mul pow5
-    let small_step = if bigint::LIMB_BITS == 32 {
+    let small_step = if bigint::Limb::BITS == 32 {
         u32_power_limit(5)
     } else {
         u64_power_limit(5)
@@ -189,7 +189,7 @@ fn karatsuba_mul_algo(big: &mut bigint::Bigint, y: &[bigint::Limb]) {
 
 #[inline(always)]
 fn new_limb(rng: &mut Rng) -> bigint::Limb {
-    if bigint::LIMB_BITS == 32 {
+    if bigint::Limb::BITS == 32 {
         rng.u32(..) as bigint::Limb
     } else {
         rng.u64(..) as bigint::Limb
diff --git a/lexical-parse-float/src/slow.rs b/lexical-parse-float/src/slow.rs
index f2b777c..30e4d52 100644
--- a/lexical-parse-float/src/slow.rs
+++ b/lexical-parse-float/src/slow.rs
@@ -649,7 +649,7 @@ pub fn byte_comp<F: RawFloat, const FORMAT: u128>(
         // After this, the numerator will be non-normalized, and the
         // denominator will be normalized. We need to add one to the
         // quotient,since we're calculating the ceiling of the divmod.
-        let (q, r) = shift.ceil_divmod(Limb::BITS);
+        let (q, r) = shift.ceil_divmod(Limb::BITS as usize);
         let r = -r;
         if r != 0 {
             num.shl_bits(r as usize).unwrap();
diff --git a/lexical-parse-float/tests/bigfloat_tests.rs b/lexical-parse-float/tests/bigfloat_tests.rs
index 02f20ee..888afcf 100644
--- a/lexical-parse-float/tests/bigfloat_tests.rs
+++ b/lexical-parse-float/tests/bigfloat_tests.rs
@@ -2,7 +2,7 @@
 
 mod stackvec;
 
-use lexical_parse_float::bigint::{Bigfloat, LIMB_BITS};
+use lexical_parse_float::bigint::{Bigfloat, Limb};
 use lexical_parse_float::float::ExtendedFloat80;
 use stackvec::vec_from_u32;
 
@@ -35,11 +35,11 @@ fn simple_test() {
     assert_eq!(&*x.data, &[0, 19531250]);
     assert_eq!(x.exp, 10);
 
-    assert_eq!(x.leading_zeros(), LIMB_BITS as u32 - 25);
+    assert_eq!(x.leading_zeros(), Limb::BITS as u32 - 25);
 
     // y has a 0 for 32-bit limbs, no 0s for 64-bit limbs.
     x *= &y;
-    let expected = if LIMB_BITS == 32 {
+    let expected = if Limb::BITS == 32 {
         vec_from_u32(&[0, 0, 0, 9765625])
     } else {
         vec_from_u32(&[0, 0, 0, 0, 9765625])
@@ -52,12 +52,12 @@ fn simple_test() {
 fn leading_zeros_test() {
     assert_eq!(Bigfloat::new().leading_zeros(), 0);
 
-    assert_eq!(Bigfloat::from_u32(0xFF).leading_zeros(), LIMB_BITS as u32 - 8);
+    assert_eq!(Bigfloat::from_u32(0xFF).leading_zeros(), Limb::BITS as u32 - 8);
     assert_eq!(Bigfloat::from_u64(0xFF00000000).leading_zeros(), 24);
 
-    assert_eq!(Bigfloat::from_u32(0xF).leading_zeros(), LIMB_BITS as u32 - 4);
+    assert_eq!(Bigfloat::from_u32(0xF).leading_zeros(), Limb::BITS as u32 - 4);
     assert_eq!(Bigfloat::from_u64(0xF00000000).leading_zeros(), 28);
 
-    assert_eq!(Bigfloat::from_u32(0xF0).leading_zeros(), LIMB_BITS as u32 - 8);
+    assert_eq!(Bigfloat::from_u32(0xF0).leading_zeros(), Limb::BITS as u32 - 8);
     assert_eq!(Bigfloat::from_u64(0xF000000000).leading_zeros(), 24);
 }
diff --git a/lexical-parse-float/tests/stackvec_tests.rs b/lexical-parse-float/tests/stackvec_tests.rs
index 40fff90..2460a78 100644
--- a/lexical-parse-float/tests/stackvec_tests.rs
+++ b/lexical-parse-float/tests/stackvec_tests.rs
@@ -2,7 +2,7 @@ mod stackvec;
 
 use core::cmp;
 
-use lexical_parse_float::bigint::{self, Limb, StackVec, LIMB_BITS};
+use lexical_parse_float::bigint::{self, Limb, StackVec};
 use stackvec::vec_from_u32;
 
 const SIZE: usize = 50;
@@ -34,7 +34,7 @@ fn simple_test() {
     assert_eq!(x.len(), 2);
     assert_eq!(x.is_empty(), false);
     assert_eq!(x.hi16(), (0x8000, true));
-    if LIMB_BITS == 32 {
+    if Limb::BITS == 32 {
         assert_eq!(x.hi32(), (0x80000002, true));
         assert_eq!(x.hi64(), (0x8000000280000000, false));
     } else {
@@ -128,7 +128,7 @@ fn math_test() {
     x.mul_small(3);
     assert_eq!(&*x, &[0, 6, 27]);
     x.mul_small(Limb::MAX);
-    let expected: VecType = if LIMB_BITS == 32 {
+    let expected: VecType = if Limb::BITS == 32 {
         vec_from_u32(&[0, 4294967290, 4294967274, 26])
     } else {
         vec_from_u32(&[0, 0, 4294967290, 4294967295, 4294967274, 4294967295, 26])
@@ -419,7 +419,7 @@ fn shl_bits_test() {
 fn shl_limbs_test() {
     let mut x = VecType::from_u32(0xD2210408);
     bigint::shl_limbs(&mut x, 2);
-    let expected: VecType = if LIMB_BITS == 32 {
+    let expected: VecType = if Limb::BITS == 32 {
         vec_from_u32(&[0, 0, 0xD2210408])
     } else {
         vec_from_u32(&[0, 0, 0, 0, 0xD2210408])

Thanks for this excellent contribution.

raldone01 · 2024-12-02T16:15:44Z

I am just fixing the tests locally.

When trying to install the cargo stuff from the development guide the cargo-count crate seems to fail to install.

❯ rustup toolchain install nightly
  rustup +nightly component add clippy
  rustup +nightly component add rustfmt
  rustup +nightly component add miri
  cargo +nightly install cargo-valgrind
  cargo +nightly install cargo-tarpaulin
  cargo +nightly install cargo-fuzz
  cargo +nightly install cargo-count
info: syncing channel updates for 'nightly-x86_64-unknown-linux-gnu'

  nightly-x86_64-unknown-linux-gnu unchanged - rustc 1.85.0-nightly (5e1440ae5 2024-12-01)

info: self-update is disabled for this build of rustup
info: any updates to rustup will need to be fetched with your system package manager
info: component 'clippy' for target 'x86_64-unknown-linux-gnu' is up to date
info: component 'rustfmt' for target 'x86_64-unknown-linux-gnu' is up to date
info: component 'miri' for target 'x86_64-unknown-linux-gnu' is up to date
    Updating crates.io index
     Ignored package `cargo-valgrind v2.2.1` is already installed, use --force to override
    Updating crates.io index
     Ignored package `cargo-tarpaulin v0.31.3` is already installed, use --force to override
    Updating crates.io index
     Ignored package `cargo-fuzz v0.12.0` is already installed, use --force to override
    Updating crates.io index
  Installing cargo-count v0.2.4
    Updating crates.io index
error: failed to compile `cargo-count v0.2.4`, intermediate artifacts can be found at `/tmp/cargo-installBQTA9A`.
To reuse those artifacts with a future compilation, set the environment variable `CARGO_TARGET_DIR` to that path.

Caused by:
  failed to select a version for the requirement `clap = "~2.11.2"`
  candidate versions found which didn't match: 4.5.21, 4.5.20, 4.5.19, ...
  location searched: crates.io index
  required by package `cargo-count v0.2.4`
  if you are looking for the prerelease package it needs to be specified explicitly
      clap = { version = "4.0.0-rc.3" }

raldone01 · 2024-12-02T16:24:32Z

The following tests fail on my machine, even on the master branch:

i128_proptest_radix
u128_proptest_radix

Should I open an issue?

Alexhuszagh · 2024-12-02T16:25:13Z

I am just fixing the tests locally.

When trying to install the cargo stuff from the development guide the cargo-count crate seems to fail to install.

❯ rustup toolchain install nightly
  rustup +nightly component add clippy
  rustup +nightly component add rustfmt
  rustup +nightly component add miri
  cargo +nightly install cargo-valgrind
  cargo +nightly install cargo-tarpaulin
  cargo +nightly install cargo-fuzz
  cargo +nightly install cargo-count
info: syncing channel updates for 'nightly-x86_64-unknown-linux-gnu'

  nightly-x86_64-unknown-linux-gnu unchanged - rustc 1.85.0-nightly (5e1440ae5 2024-12-01)

info: self-update is disabled for this build of rustup
info: any updates to rustup will need to be fetched with your system package manager
info: component 'clippy' for target 'x86_64-unknown-linux-gnu' is up to date
info: component 'rustfmt' for target 'x86_64-unknown-linux-gnu' is up to date
info: component 'miri' for target 'x86_64-unknown-linux-gnu' is up to date
    Updating crates.io index
     Ignored package `cargo-valgrind v2.2.1` is already installed, use --force to override
    Updating crates.io index
     Ignored package `cargo-tarpaulin v0.31.3` is already installed, use --force to override
    Updating crates.io index
     Ignored package `cargo-fuzz v0.12.0` is already installed, use --force to override
    Updating crates.io index
  Installing cargo-count v0.2.4
    Updating crates.io index
error: failed to compile `cargo-count v0.2.4`, intermediate artifacts can be found at `/tmp/cargo-installBQTA9A`.
To reuse those artifacts with a future compilation, set the environment variable `CARGO_TARGET_DIR` to that path.

Caused by:
  failed to select a version for the requirement `clap = "~2.11.2"`
  candidate versions found which didn't match: 4.5.21, 4.5.20, 4.5.19, ...
  location searched: crates.io index
  required by package `cargo-count v0.2.4`
  if you are looking for the prerelease package it needs to be specified explicitly
      clap = { version = "4.0.0-rc.3" }

That's fine, it seems cargo-count requires an older version of nightly. I can update the documentation later.

As long as ci/test.sh and ci/check.sh pass, it should be good to go. I'll update the documentation on this. If there's a failure in fast_div, that's fine: I'm correcting that later. It's just for algorithm assessment and is never used in the actual code. I should remove that from the default test suite later.

Alexhuszagh · 2024-12-02T16:26:32Z

The following tests fail on my machine, even on the master branch:
i128_proptest_radix
u128_proptest_radix
Should I open an issue?

That's already an open issue, I'm fixing those currently. I've been super swamped with the holidays and a large number of complications at work so I'm behind schedule. I'll have a quick fix and a high-performant fix soon enough.

raldone01 · 2024-12-02T16:36:20Z

The tests should be fine now.

Alexhuszagh · 2024-12-02T16:42:14Z

I'll fix the lint, don't worry about that.

raldone01 · 2024-12-02T16:42:41Z

Thanks.
Format on save in vscode disagrees with cargo +nightly fmt -- --check.
That slipped woops.

raldone01 · 2024-12-02T16:44:41Z

Wait I fixed the settings.json so that it doesn't disagree.

raldone01 · 2024-12-02T17:09:13Z

Wait the settings json is not quite correct.

Alexhuszagh · 2024-12-02T17:13:32Z

Wait the settings json is not quite correct.

Let me know when all changes have been made and it should be good to merge. Once again thank you for this excellent contribution.

Update the vscode settings so that rust analyzer and `cargo +nightly fmt -- --check` agree. Minor spelling.

raldone01 · 2024-12-02T17:18:41Z

OK now the settings are correct. There was an explicit warning not to use cargo fmt. Luckily rustfmt also takes +nightly.

This pr should be done now.
Thanks for your guidance and patience.

raldone01 · 2024-12-02T17:45:20Z

Yay! It's merged. 🚀

I rewrote some of the Python scripts that generate the lookup tables in rust and hooked them into the build.rs to regenerate the files automatically on deletion.

Would you be interested in these changes too?

Alexhuszagh · 2024-12-02T17:47:22Z

Yay! It's merged. 🚀

I rewrote some of the Python scripts that generate the lookup tables in rust and hooked them into the build.rs to regenerate the files automatically on deletion.

Would you be interested in these changes too?

Absolutely. If the changes make the table generation easier to maintain and produce the same output, contributions are greatly appreciated :D.

This better documents the development requirements and also the algorithm assessment logic. Related to #180.

raldone01 force-pushed the minor-spelling branch 4 times, most recently from 65659ee to dab0e28 Compare November 28, 2024 20:30

Alexhuszagh self-assigned this Dec 2, 2024

Alexhuszagh added documentation Fix in the documentation good first issue Good for newcomers labels Dec 2, 2024

raldone01 force-pushed the minor-spelling branch from dab0e28 to 5430dba Compare December 2, 2024 16:34

raldone01 force-pushed the minor-spelling branch from 5430dba to cc76e85 Compare December 2, 2024 16:52

Replace LIMB_BITS with the modern BITS constant.

1986bc4

Update the vscode settings so that rust analyzer and `cargo +nightly fmt -- --check` agree. Minor spelling.

raldone01 force-pushed the minor-spelling branch from cc76e85 to 1986bc4 Compare December 2, 2024 17:15

Alexhuszagh merged commit 8128624 into Alexhuszagh:main Dec 2, 2024
32 checks passed

Alexhuszagh added a commit that referenced this pull request Dec 3, 2024

Update development guides.

ea30b9a

This better documents the development requirements and also the algorithm assessment logic. Related to #180.

Alexhuszagh mentioned this pull request Dec 3, 2024

Update development guides. #181

Merged

Alexhuszagh added a commit that referenced this pull request Dec 3, 2024

Update development guides.

cbeeebe

This better documents the development requirements and also the algorithm assessment logic. Related to #180.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Minor spelling and add deprecation notice to `LIMB_BITS`. #180

Minor spelling and add deprecation notice to `LIMB_BITS`. #180

raldone01 commented Nov 28, 2024 •

edited

Loading

raldone01 commented Dec 2, 2024 •

edited

Loading

Alexhuszagh commented Dec 2, 2024 •

edited

Loading

Alexhuszagh commented Dec 2, 2024

raldone01 commented Dec 2, 2024

raldone01 commented Dec 2, 2024 •

edited

Loading

Alexhuszagh commented Dec 2, 2024 •

edited

Loading

Alexhuszagh commented Dec 2, 2024 •

edited

Loading

raldone01 commented Dec 2, 2024

Alexhuszagh commented Dec 2, 2024

raldone01 commented Dec 2, 2024

raldone01 commented Dec 2, 2024

raldone01 commented Dec 2, 2024

Alexhuszagh commented Dec 2, 2024

raldone01 commented Dec 2, 2024 •

edited

Loading

raldone01 commented Dec 2, 2024

Alexhuszagh commented Dec 2, 2024

Minor spelling and add deprecation notice to LIMB_BITS. #180

Minor spelling and add deprecation notice to LIMB_BITS. #180

Conversation

raldone01 commented Nov 28, 2024 • edited Loading

raldone01 commented Dec 2, 2024 • edited Loading

Alexhuszagh commented Dec 2, 2024 • edited Loading

Alexhuszagh commented Dec 2, 2024

raldone01 commented Dec 2, 2024

raldone01 commented Dec 2, 2024 • edited Loading

Alexhuszagh commented Dec 2, 2024 • edited Loading

Alexhuszagh commented Dec 2, 2024 • edited Loading

raldone01 commented Dec 2, 2024

Alexhuszagh commented Dec 2, 2024

raldone01 commented Dec 2, 2024

raldone01 commented Dec 2, 2024

raldone01 commented Dec 2, 2024

Alexhuszagh commented Dec 2, 2024

raldone01 commented Dec 2, 2024 • edited Loading

raldone01 commented Dec 2, 2024

Alexhuszagh commented Dec 2, 2024

Minor spelling and add deprecation notice to `LIMB_BITS`. #180

Minor spelling and add deprecation notice to `LIMB_BITS`. #180

raldone01 commented Nov 28, 2024 •

edited

Loading

raldone01 commented Dec 2, 2024 •

edited

Loading

Alexhuszagh commented Dec 2, 2024 •

edited

Loading

raldone01 commented Dec 2, 2024 •

edited

Loading

Alexhuszagh commented Dec 2, 2024 •

edited

Loading

Alexhuszagh commented Dec 2, 2024 •

edited

Loading

raldone01 commented Dec 2, 2024 •

edited

Loading