Skip to content

Commit

Permalink
Merge pull request #185 from Alexhuszagh/169-bug-unit-tests-failures
Browse files Browse the repository at this point in the history
This fixes issues with writing digits with custom radices.

This calculates the digit count prior to writing the digits, so the digits are always written at the front rather than copying the written digits into place. Additional performance enhancements were added for base 2 and base 4 radices.

Closes #169
  • Loading branch information
Alexhuszagh authored Dec 5, 2024
2 parents a1b993f + e11c0ec commit 8f21471
Show file tree
Hide file tree
Showing 19 changed files with 869 additions and 388 deletions.
15 changes: 8 additions & 7 deletions ci/comprehensive.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ cargo --version
script_dir=$(dirname "${BASH_SOURCE[0]}")
script_home=$(realpath "${script_dir}")
home=$(dirname "${script_home}")
version="${CARGO_VERSION}"
cd "${home}"

# Ensure we have all our testing data files
Expand All @@ -20,16 +21,16 @@ run_tests() {
# Test the parse-float correctness tests
cd "${home}"
cd lexical-parse-float/etc/correctness
cargo run "${@}" --release --bin test-parse-golang
cargo run "${@}" --release --bin test-parse-golang --features digit-separator
cargo run "${@}" --release --bin test-parse-unittests
cargo ${version} run "${@}" --release --bin test-parse-golang
cargo ${version} run "${@}" --release --bin test-parse-golang --features digit-separator
cargo ${version} run "${@}" --release --bin test-parse-unittests

# Test the write-float correctness tests.
cd "${home}"
cd lexical-write-float/etc/correctness
cargo run "${@}" --release --bin shorter_interval
cargo run "${@}" --release --bin random
cargo run "${@}" --release --bin simple_random -- --iterations 1000000
cargo ${version} run "${@}" --release --bin shorter_interval
cargo ${version} run "${@}" --release --bin random
cargo ${version} run "${@}" --release --bin simple_random -- --iterations 1000000
}

run_tests
Expand All @@ -41,5 +42,5 @@ if [ ! -z "${EXHAUSTIVE}" ]; then
# Test the parse-float correctness tests
cd "${home}"
cd lexical-parse-float/etc/correctness
cargo run "${@}" --release --bin test-parse-random
cargo ${version} run "${@}" --release --bin test-parse-random
fi
3 changes: 2 additions & 1 deletion ci/miri.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ set -ex
script_dir=$(dirname "${BASH_SOURCE[0]}")
script_home=$(realpath "${script_dir}")
home=$(dirname "${script_home}")
version="${CARGO_VERSION}"
cd "${home}"

# Print our cargo version, for debugging.
cargo --version
cargo ${version} --version

# Ensure we have all our testing data files
git submodule update --init
Expand Down
45 changes: 25 additions & 20 deletions ci/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ set -ex
script_dir=$(dirname "${BASH_SOURCE[0]}")
script_home=$(realpath "${script_dir}")
home=$(dirname "${script_home}")
version="${CARGO_VERSION}"
cd "${home}"

# Print our cargo version, for debugging.
cargo --version
cargo ${version} --version

# Ensure we have all our benchmark data files
git submodule update --init lexical-benchmark/data
Expand All @@ -37,7 +38,7 @@ FEATURES=(

check_error() {
local feature=$1
if 2>/dev/null cargo check --no-default-features --features="${feature}" ; then
if 2>/dev/null cargo ${version} check --no-default-features --features="${feature}" ; then
>&2 echo "The feature ${feature} did not error..."
exit 1
fi
Expand All @@ -52,21 +53,21 @@ check() {
# Need to test a few permutations just to ensure everything compiles.
for features in "${FEATURES[@]}"; do
check_features="$DEFAULT_FEATURES --features=$REQUIRED_FEATURES,$features"
cargo check --tests $check_features
cargo ${version} check --tests $check_features
done

# Check each of our sub-crates compiles.
cd lexical-parse-float
cargo check --tests
cargo ${version} check --tests

cd ../lexical-parse-integer
cargo check --tests
cargo ${version} check --tests

cd ../lexical-write-float
cargo check --tests
cargo ${version} check --tests

cd ../lexical-write-integer
cargo check --tests
cargo ${version} check --tests

# ensure our partial features aren't allowed, as are unsupported features
cd ../lexical-core
Expand All @@ -86,8 +87,8 @@ check() {
# Build target.
build() {
build_features="$DEFAULT_FEATURES --features=$REQUIRED_FEATURES"
cargo build $build_features
cargo build $build_features --release
cargo ${version} build $build_features
cargo ${version} build $build_features --release
}

# Test target.
Expand All @@ -101,24 +102,28 @@ test() {

# Default tests.
test_features="$DEFAULT_FEATURES --features=$REQUIRED_FEATURES"
cargo test $test_features $DOCTESTS
cargo test $test_features $DOCTESTS --release
cargo test --features=radix,format,compact $DOCTESTS --release
cargo ${version} test $test_features $DOCTESTS
cargo ${version} test $test_features $DOCTESTS --release
cargo ${version} test --features=radix,format,compact $DOCTESTS --release
# NOTE: This tests a regressions, related to #96.
cargo test --features=format $DOCTESTS
cargo ${version} test --features=format $DOCTESTS

# Ensure we test radix without the compact feature
# See #169
cargo ${version} test --features=radix,format --release

# this fixes an issue where the lexical and lexical-core tests weren't being run
cd lexical-core
cargo test $test_features,format
cargo test $test_features,radix
cargo test $test_features,format,radix
cargo ${version} test $test_features,format
cargo ${version} test $test_features,radix
cargo ${version} test $test_features,format,radix
cd ..

# this fixes an issue where the lexical and lexical-core tests weren't being run
cd lexical
cargo test $test_features,format
cargo test $test_features,radix
cargo test $test_features,format,radix
cargo ${version} test $test_features,format
cargo ${version} test $test_features,radix
cargo ${version} test $test_features,format,radix
cd ..
}

Expand All @@ -140,7 +145,7 @@ bench() {

cd lexical-benchmark
bench_features="$DEFAULT_FEATURES --features=$REQUIRED_FEATURES"
cargo test $bench_features --bench '*'
cargo ${version} test $bench_features --bench '*'
cd ..
}

Expand Down
10 changes: 5 additions & 5 deletions lexical-write-float/src/algorithm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use lexical_util::bf16::bf16;
use lexical_util::f16::f16;
use lexical_util::format::{NumberFormat, STANDARD};
use lexical_util::num::{AsPrimitive, Float};
use lexical_write_integer::decimal::DigitCount;
use lexical_write_integer::decimal::DecimalCount;
use lexical_write_integer::write::WriteInteger;

use crate::float::{ExtendedFloat80, RawFloat};
Expand Down Expand Up @@ -761,7 +761,7 @@ pub fn compute_right_closed_directed<F: RawFloat>(float: F, shorter: bool) -> Ex
#[allow(clippy::branches_sharing_code)] // reason="could differentiate later"
pub fn write_digits_u32(bytes: &mut [u8], mantissa: u32) -> usize {
debug_assert!(bytes.len() >= 10);
mantissa.write_mantissa::<u32, { STANDARD }>(bytes)
mantissa.write_mantissa::<{ STANDARD }>(bytes)
}

/// Write the significant digits, when the significant digits cannot fit in a
Expand All @@ -774,7 +774,7 @@ pub fn write_digits_u32(bytes: &mut [u8], mantissa: u32) -> usize {
#[allow(clippy::branches_sharing_code)] // reason="could differentiate later"
pub fn write_digits_u64(bytes: &mut [u8], mantissa: u64) -> usize {
debug_assert!(bytes.len() >= 20);
mantissa.write_mantissa::<u64, { STANDARD }>(bytes)
mantissa.write_mantissa::<{ STANDARD }>(bytes)
}

// EXTENDED
Expand Down Expand Up @@ -1218,7 +1218,7 @@ impl DragonboxFloat for f32 {
#[inline(always)]
fn digit_count(mantissa: u64) -> usize {
debug_assert!(mantissa <= u32::MAX as u64);
(mantissa as u32).digit_count()
(mantissa as u32).decimal_count()
}

#[inline(always)]
Expand Down Expand Up @@ -1328,7 +1328,7 @@ impl DragonboxFloat for f64 {

#[inline(always)]
fn digit_count(mantissa: u64) -> usize {
mantissa.digit_count()
mantissa.decimal_count()
}

#[inline(always)]
Expand Down
6 changes: 3 additions & 3 deletions lexical-write-float/src/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ where
let shl = calculate_shl(exp, bits_per_digit);
let value = mantissa << shl;

let count = value.write_mantissa::<M, FORMAT>(&mut bytes[1..]);
let count = value.write_mantissa::<FORMAT>(&mut bytes[1..]);
bytes[0] = bytes[1];
bytes[1] = decimal_point;
let zeros = rtrim_char_count(&bytes[2..count + 1], b'0');
Expand Down Expand Up @@ -221,7 +221,7 @@ where

// Won't panic, if the buffer is large enough to hold the significant
// digits.
let count = value.write_mantissa::<M, FORMAT>(&mut bytes[cursor..]);
let count = value.write_mantissa::<FORMAT>(&mut bytes[cursor..]);
let zeros = rtrim_char_count(&bytes[cursor..cursor + count], b'0');
let digit_count = count - zeros;
cursor += digit_count;
Expand Down Expand Up @@ -267,7 +267,7 @@ where
let shl = calculate_shl(exp, bits_per_digit);
let value = mantissa << shl;

let count = value.write_mantissa::<M, FORMAT>(bytes);
let count = value.write_mantissa::<FORMAT>(bytes);
let zeros = rtrim_char_count(&bytes[..count], b'0');
let mut digit_count = count - zeros;

Expand Down
2 changes: 1 addition & 1 deletion lexical-write-float/src/hex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ where
let shl = calculate_shl(exp, bits_per_digit);
let value = mantissa << shl;

let count = value.write_mantissa::<M, FORMAT>(&mut bytes[1..]);
let count = value.write_mantissa::<FORMAT>(&mut bytes[1..]);
bytes[0] = bytes[1];
bytes[1] = decimal_point;
let zeros = rtrim_char_count(&bytes[2..count + 1], b'0');
Expand Down
2 changes: 1 addition & 1 deletion lexical-write-float/src/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ pub fn write_exponent<const FORMAT: u128>(
bytes[*cursor] = exponent_character;
*cursor += 1;
let positive_exp: u32 = write_exponent_sign::<FORMAT>(bytes, cursor, exp);
*cursor += positive_exp.write_exponent::<u32, FORMAT>(&mut bytes[*cursor..]);
*cursor += positive_exp.write_exponent::<FORMAT>(&mut bytes[*cursor..]);
}

/// Detect the notation to use for the float formatter and call the appropriate
Expand Down
1 change: 1 addition & 0 deletions lexical-write-integer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ features = ["write-integers"]
# Fix: https://github.com/BurntSushi/quickcheck/pull/296
quickcheck = { git = "https://github.com/Alexhuszagh/quickcheck/", branch = "i32min-shrink-bound-legacy" }
proptest = ">=1.5.0"
rustversion = ">=1.0.18"

[features]
default = ["std"]
Expand Down
Loading

0 comments on commit 8f21471

Please sign in to comment.