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

Implement Roots for BigInt and BigUint #56

Merged
merged 4 commits into from
Jul 19, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ name = "shootout-pidigits"
[dependencies]

[dependencies.num-integer]
version = "0.1.38"
version = "0.1.39"
default-features = false

[dependencies.num-traits]
Expand Down
32 changes: 31 additions & 1 deletion benches/bigint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
extern crate test;
extern crate num_bigint;
extern crate num_traits;
extern crate num_integer;
extern crate rand;

use std::mem::replace;
use test::Bencher;
use num_bigint::{BigInt, BigUint, RandBigInt};
use num_traits::{Zero, One, FromPrimitive, Num};
use rand::{SeedableRng, StdRng};
use rand::{SeedableRng, StdRng, Rng};

fn get_rng() -> StdRng {
let mut seed = [0; 32];
Expand Down Expand Up @@ -342,3 +343,32 @@ fn modpow_even(b: &mut Bencher) {

b.iter(|| base.modpow(&e, &m));
}

#[bench]
fn roots_sqrt(b: &mut Bencher) {
let mut rng = get_rng();
let x = rng.gen_biguint(2048);

b.iter(|| x.sqrt());
}

#[bench]
fn roots_cbrt(b: &mut Bencher) {
let mut rng = get_rng();
let x = rng.gen_biguint(2048);

b.iter(|| x.cbrt());
}

#[bench]
fn roots_nth(b: &mut Bencher) {
let mut rng = get_rng();
let x = rng.gen_biguint(2048);
// Although n is u32, here we limit it to the set of u8 values since it
// hugely impacts the performance of nth_root due to exponentiation to
// the power of n-1. Using very large values for n is also not very realistic,
// and any n > x's bit size produces 1 as a result anyway.
let n: u8 = rng.gen();
Copy link
Member

Choose a reason for hiding this comment

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

This is from a seeded RNG, so what value do you get? I think you might as well just pick a reasonable n directly. Random x is still fine.


b.iter(|| { x.nth_root(n as u32) });
}
30 changes: 29 additions & 1 deletion src/bigint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::iter::{Product, Sum};
#[cfg(feature = "serde")]
use serde;

use integer::Integer;
use integer::{Integer, Roots};
use traits::{ToPrimitive, FromPrimitive, Num, CheckedAdd, CheckedSub,
CheckedMul, CheckedDiv, Signed, Zero, One};

Expand Down Expand Up @@ -1802,6 +1802,15 @@ impl Integer for BigInt {
}
}

impl Roots for BigInt {
fn nth_root(&self, n: u32) -> Self {
assert!(!(self.is_negative() && n.is_even()),
"n-th root is undefined for number (n={})", n);

BigInt::from_biguint(self.sign, self.data.nth_root(n))
}
}

impl ToPrimitive for BigInt {
#[inline]
fn to_i64(&self) -> Option<i64> {
Expand Down Expand Up @@ -2538,6 +2547,25 @@ impl BigInt {
};
BigInt::from_biguint(sign, mag)
}

/// Returns the truncated principal square root of `self` --
/// see [Roots::sqrt](Roots::sqrt).
// struct.BigInt.html#trait.Roots
Copy link
Member

Choose a reason for hiding this comment

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

Another stray link? And I think all of these links like [Roots::sqrt](Roots::sqrt) won't actually work, AFAICS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I run cargo doc --no-deps --open and navigate to the page of BigInt or BigUint structs, paths of the form Roots::sqrt are rendered as links to https://docs.rs/num-integer/0.1/num_integer/trait.Roots.html#method.sqrt - I'm a little confused, is this not what we want? I was merely following this RFC, but I could be missing something.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, it seems that works on nightly, but not beta or stable rustc yet. We should make those explicit for now.

pub fn sqrt(&self) -> Self {
Roots::sqrt(self)
}

/// Returns the truncated principal cube root of `self` --
/// see [Roots::cbrt](Roots::cbrt).
pub fn cbrt(&self) -> Self {
Roots::cbrt(self)
}

/// Returns the truncated principal `n`th root of `self` --
/// See [Roots::nth_root](Roots::nth_root).
pub fn nth_root(&self, n: u32) -> Self {
Roots::nth_root(self, n)
}
}

impl_sum_iter_type!(BigInt);
Expand Down
69 changes: 67 additions & 2 deletions src/biguint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ use std::ascii::AsciiExt;
#[cfg(feature = "serde")]
use serde;

use integer::Integer;
use integer::{Integer, Roots};
use traits::{ToPrimitive, FromPrimitive, Float, Num, Unsigned, CheckedAdd, CheckedSub, CheckedMul,
CheckedDiv, Zero, One};
CheckedDiv, Zero, One, pow};

use big_digit::{self, BigDigit, DoubleBigDigit};

Expand Down Expand Up @@ -1026,6 +1026,52 @@ impl Integer for BigUint {
}
}

impl Roots for BigUint {
fn nth_root(&self, n: u32) -> Self {
Copy link
Member

Choose a reason for hiding this comment

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

I think it would probably be valuable to have dedicated sqrt and cbrt implementations -- did you try it? Maybe benchmarks would prove me wrong, but we can also wait until later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here are the benchmark results:

test roots_cbrt            ... bench:      48,670 ns/iter (+/- 1,322)
test roots_cbrt_nth_root_3 ... bench:      52,285 ns/iter (+/- 1,541)
test roots_sqrt            ... bench:      53,752 ns/iter (+/- 4,954)
test roots_sqrt_nth_root_2 ... bench:      57,622 ns/iter (+/- 1,585)

roots_cbrt and roots_sqrt test the optimized versions - I took sqrt from #51 and adjusted it for cbrt. The other two benchmarks just called nth_root(2) and nth_root(3) - of course, for the sake of benchmark, I didn't forward the calls to nth_root with n=2 and n=3 to the optimized sqrt and cbrt, but I'll definitely do that, since I see this is how it's done in impls for primitive types as well.

I would prefer to incorporate the dedicated implementations in this PR, although I'm struggling a little since I'm not particularly fond of having parts of nth_root, sqrt, cbrt replicated in all the methods. But I'm not too sure what (if anything) would be best to do about it 🙂

assert!(n > 0, "n must be at least 1");

let one = BigUint::one();

// Trivial cases
if self.is_zero() {
return BigUint::zero();
}

if self.is_one() {
return one;
}

let n = n as usize;
let n_min_1 = (n as usize) - 1;
Copy link
Member

Choose a reason for hiding this comment

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

Don't need to cast n as usize twice.


// Newton's method to compute the nth root of an integer.
//
// Reference:
// Brent & Zimmermann, Modern Computer Arithmetic, v0.5.9, Algorithm 1.14
//
// Set initial guess to something definitely >= floor(nth_root of self)
// but as low as possible to speed up convergence.
let bit_len = self.len() * big_digit::BITS;
let guess = one << (bit_len/n + 1);

let mut u = guess;
let mut s: BigUint;

loop {
s = u;
let q = self / pow(s.clone(), n_min_1);
let t: BigUint = n_min_1 * &s + q;

// Compute the candidate value for next iteration
u = t / n;

if u >= s { break; }
}

s
}
}

fn high_bits_to_u64(v: &BigUint) -> u64 {
match v.data.len() {
0 => 0,
Expand Down Expand Up @@ -1749,6 +1795,25 @@ impl BigUint {
}
acc
}

/// Returns the truncated principal square root of `self` --
/// see [Roots::sqrt](Roots::sqrt).
// struct.BigInt.html#trait.Roots
Copy link
Member

Choose a reason for hiding this comment

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

These links don't look right -- did you forget to complete this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eh, forgot to remove.

pub fn sqrt(&self) -> Self {
Roots::sqrt(self)
}

/// Returns the truncated principal cube root of `self` --
/// see [Roots::cbrt](Roots::cbrt).
pub fn cbrt(&self) -> Self {
Roots::cbrt(self)
}

/// Returns the truncated principal `n`th root of `self` --
/// See [Roots::nth_root](Roots::nth_root).
pub fn nth_root(&self, n: u32) -> Self {
Roots::nth_root(self, n)
}
}

/// Returns the number of least-significant bits that are zero,
Expand Down
84 changes: 84 additions & 0 deletions tests/roots.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
extern crate num_bigint;
extern crate num_integer;
extern crate num_traits;

mod biguint {
use num_bigint::BigUint;
use num_traits::FromPrimitive;
use std::str::FromStr;

fn check(x: i32, n: u32, expected: i32) {
let big_x: BigUint = FromPrimitive::from_i32(x).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

If you use an unsigned type instead of i32, then From makes this simpler -- BigUint::from(x).

let big_expected: BigUint = FromPrimitive::from_i32(expected).unwrap();

assert_eq!(big_x.nth_root(n), big_expected);
}

#[test]
fn test_sqrt() {
check(99, 2, 9);
check(100, 2, 10);
check(120, 2, 10);
}

#[test]
fn test_cbrt() {
check(8, 3, 2);
check(26, 3, 2);
}

#[test]
fn test_nth_root() {
check(0, 1, 0);
check(10, 1, 10);
check(100, 4, 3);
}

#[test]
#[should_panic]
fn test_nth_root_n_is_zero() {
check(4, 0, 0);
}

#[test]
fn test_nth_root_big() {
let x: BigUint = FromStr::from_str("123_456_789").unwrap();
let expected : BigUint = FromPrimitive::from_i32(6).unwrap();

assert_eq!(x.nth_root(10), expected);
}
}

mod bigint {
use num_bigint::BigInt;
use num_traits::FromPrimitive;

fn check(x: i32, n: u32, expected: i32) {
let big_x: BigInt = FromPrimitive::from_i32(x).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Similarly, BigInt::from(x) should be fine.

let big_expected: BigInt = FromPrimitive::from_i32(expected).unwrap();

assert_eq!(big_x.nth_root(n), big_expected);
}

#[test]
fn test_nth_root() {
check(-100, 3, -4);
}

#[test]
#[should_panic]
fn test_nth_root_x_neg_n_even() {
check(-100, 4, 0);
}

#[test]
#[should_panic]
fn test_sqrt_x_neg() {
check(-4, 2, -2);
}

#[test]
fn test_cbrt() {
check(-8, 3, -2);
}
}