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

Enforce rounding in rounded_arith_opp to fix incorrect behaviour. #4

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

Conversation

nafur
Copy link

@nafur nafur commented Jun 24, 2014

A discussion between Florian ([email protected]) and Guillaume ([email protected]) lead to the proposed patch. Compiling the code stated below with clang++ -std=c++11 -O1 leads to incorrect behaviour. (arch linux x64, clang 3.4.2, boost 1.55.0)
In a nutshell, we compare the results for 41 * 0.1 and -(-41 * 0.1) and the resulting intervals turn out to be disjoint, none of them containing 4.1. Note that tiny changes to the code (for example changing the order of the assignments) can make the error vanish.

Guillaume proposed to either switch to rounded_transc_std or add this->force_rounding to all occurrences of (-x) after a deeper look into the generated assembly. (Thanks for this!) So here is a patch for this change.

#include <iostream>
#include <iomanip>
#include <boost/numeric/interval.hpp>

using namespace boost::numeric;
using namespace boost::numeric::interval_lib;

typedef save_state<rounded_transc_opp<double>> rnd;
typedef checking_no_nan<double, checking_no_nan<double>> chk;
typedef interval<double, policies<rnd, chk>> Interval;

std::ostream& operator<<(std::ostream& os, const Interval& i) {
    return os << std::setprecision(1000) << "[" << i.lower() << ", " << i.upper() << "]";
}

int main() {
    Interval I1( double(41) );
    Interval I2( -double(41) );
    Interval M = Interval( double(1) ) / Interval( double(10) );
    Interval R1 = I1 * M;
    Interval R2 = -(I2 * M);

    if (R1.lower() > R2.upper() || R1.lower() > R2.upper()) {
        std::cout << "Error: the intervals" << std::endl;
        std::cout << "\t   41 * 0.1  = " << R1 << std::endl;
        std::cout << "\t-(-41 * 0.1) = " << R2 << std::endl;
        std::cout << "\tare disjoint!" << std::endl;
    }
    return 0;
}

Using clang with optimizations leads to incorrect results under certain circumstances.
@mcquay239
Copy link

Hello, @mclow, please, merge this PR. It's still relevant because of MSVC (still reproduced on MSVC 2015) over-optimization, see https://svn.boost.org/trac/boost/ticket/12018. Thank you!

@jeking3
Copy link
Contributor

jeking3 commented May 17, 2018

I am adding a CI environment so we can better handle these requests and prove quality of changes.

@jeking3
Copy link
Contributor

jeking3 commented Oct 30, 2018

This breaks test_add:

testing.capture-output ../../../bin.v2/libs/numeric/interval/test/add.test/gcc-7.3/debug/cxxstd-03-iso/visibility-hidden/add.run
====== BEGIN OUTPUT ======
/boost/libs/numeric/interval/test/add.cpp(223): test (test_add<I2>()) failed in function: 'int test_main(int, char**)'
/boost/libs/numeric/interval/test/add.cpp(225): test (test_add1<I2>()) failed in function: 'int test_main(int, char**)'
/boost/libs/numeric/interval/test/add.cpp(227): test (test_add2<I2>()) failed in function: 'int test_main(int, char**)'
/boost/libs/numeric/interval/test/add.cpp(235): test (test_addeq<I2>()) failed in function: 'int test_main(int, char**)'
/boost/libs/numeric/interval/test/add.cpp(237): test (test_addeq1<I2>()) failed in function: 'int test_main(int, char**)'

**** 5 errors detected

EXIT STATUS: 201
====== END OUTPUT ======

Copy link
Contributor

@jeking3 jeking3 left a comment

Choose a reason for hiding this comment

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

Please rebase on develop.

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.

3 participants