Skip to content

simple_continued_fraction: added public functions to access and modify the coefficients #971

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

Open
Tomaqa wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

simple_continued_fraction: added public functions to access and modify the coefficients #971

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
08f0490
Add free function for calculation of simple continued fraction coeffs
mborland Apr 4, 2023
b547ac7
Update docs
mborland Apr 4, 2023
1f3a079
Fix casting for MSVC
mborland Apr 4, 2023
d83bd53
Typedef for Z `int_type`; replaced member variable `x_` with `precisi…
Apr 11, 2023
25ab7c5
Implemented `simple_continued_fraction(std::vector<Z>)`
Apr 11, 2023
c4c4592
Implemented templated free function `Rational to_rational(simple_cont…
Apr 11, 2023
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
Prev Previous commit
Next Next commit
Implemented simple_continued_fraction(std::vector<Z>)
  • Loading branch information
Tomáš Kolárik committed Apr 11, 2023
commit 25ab7c57c889fe3194198fc1d7a0c145ff2932a3
57 changes: 39 additions & 18 deletions include/boost/math/tools/simple_continued_fraction.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,34 @@ class simple_continued_fraction {
public:
typedef Z int_type;

simple_continued_fraction(Real x) {
simple_continued_fraction(std::vector<Z> data) : b_{std::move(data)} {
Copy link
Member

@mborland mborland Apr 12, 2023

Choose a reason for hiding this comment

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

Suggested change
simple_continued_fraction(std::vector<Z> data) : b_{std::move(data)} {
simple_continued_fraction(std::vector<Z>&& data) : b_{data} {

As written this makes a copy of the data vector and then moves it into b_. I expect you want to take an r-value reference instead.

Copy link
Author

@Tomaqa Tomaqa Apr 12, 2023

Choose a reason for hiding this comment

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

simple_continued_fraction(std::vector<Z>) can be used both with std::vector<Z>&& and const std::vector<Z>& arguments, whereas simple_continued_fraction(std::vector<Z>&&) cannot be used with const std::vector<Z>&. In the case of std::vector<Z>&& argument, there should no significant difference (std::vector<Z> can require one more call to move constructor, which is negligible).

More importantly, I'm pretty sure that your suggestion simple_continued_fraction(std::vector<Z>&& data) : b_{data} will call copy constructor on data, because you pass it as an lvalue, not as an rvalue.

const size_t size_ = b_.size();
if (size_ == 0) {
throw std::length_error("Array of coefficients is empty.");
}

for (size_t i = 1; i < size_; ++i) {
if (b_[i] <= 0) {
std::ostringstream oss;
oss << "Found a negative partial denominator: b[" << i << "] = " << b_[i] << ".";
throw std::domain_error(oss.str());
}
}

canonicalize();
}

simple_continued_fraction(Real x) : b_{} {
using std::floor;
using std::abs;
using std::sqrt;
using std::isfinite;
const Real orig_x = x;
if (!isfinite(x)) {
throw std::domain_error("Cannot convert non-finites into continued fractions.");
}

constexpr int p = std::numeric_limits<Real>::max_digits10;
if constexpr (p == 2147483647) {
precision_ = orig_x.backend().precision();
} else {
precision_ = p;
if constexpr (std_precision == 2147483647) {
precision_ = x.backend().precision();
}

b_.reserve(50);
Expand All @@ -61,6 +74,8 @@ class simple_continued_fraction {
b_.shrink_to_fit();
return;
}

const Real orig_x = x;
x = 1/(x-bj);
Real f = bj;
if (bj == 0) {
Expand All @@ -87,14 +102,7 @@ class simple_continued_fraction {
D = 1/D;
f *= (C*D);
}
// Deal with non-uniqueness of continued fractions: [a0; a1, ..., an, 1] = a0; a1, ..., an + 1].
// The shorter representation is considered the canonical representation,
// so if we compute a non-canonical representation, change it to canonical:
if (b_.size() > 2 && b_.back() == 1) {
b_.pop_back();
b_.back() += 1;
}
b_.shrink_to_fit();
canonicalize();

const size_t size_ = b_.size();
for (size_t i = 1; i < size_; ++i) {
Expand Down Expand Up @@ -163,9 +171,22 @@ class simple_continued_fraction {
template<typename T, typename Z2>
friend std::ostream& operator<<(std::ostream& out, simple_continued_fraction<T, Z2>& scf);
private:
std::vector<Z> b_{};
static constexpr int std_precision = std::numeric_limits<Real>::max_digits10;

void canonicalize() {
// Deal with non-uniqueness of continued fractions: [a0; a1, ..., an, 1] = a0; a1, ..., an + 1].
// The shorter representation is considered the canonical representation,
// so if we compute a non-canonical representation, change it to canonical:
if (b_.size() > 2 && b_.back() == 1) {
b_.pop_back();
b_.back() += 1;
}
b_.shrink_to_fit();
}

std::vector<Z> b_;

int precision_{};
int precision_{std_precision};
};


Expand All @@ -189,7 +210,7 @@ std::ostream& operator<<(std::ostream& out, simple_continued_fraction<Real, Z2>&
template<typename Real, typename Z = std::int64_t>
inline auto simple_continued_fraction_coefficients(Real x)
{
auto temp = simple_continued_fraction(x);
auto temp = simple_continued_fraction<Real, Z>(x);
return temp.get_data();
}

Expand Down