changed max_precision for real #18

SagnikDey92 · 2019-04-09T14:31:31Z

Currently only the greater than and less than operator tests don't pass. They will need similar changes as in the == operator test. That is: remove precision exception checks where a comparison is possible and just check the functioning of < operator. I also added a few tests under My Tests section in the real eq operator test. I didn't make the changes in the <, > operator tests as I wanted to confirm whether this is correct. If yes, I'll update once again. All other tests except these two are still passing. These two are the real_lower_than_operator_test and real_greater_than_operator_test.

Edit: Okay, I was being lazy not updating the greater than and less than tests. I've now changed them for the travis build to pass.

Laouen

I've put some comments, there is a general conceptual error that is spread in the code and tests in general. I didn't comment on all the errors because most of them are the same error and it makes no point to comment the same all over the code.

The main problem is that a maximum precision is set to determine until where the library will iterate the precision in order to get more accurate approximation intervals that could possibly answer the operator question ( ==, <, >, etc). But if the maximum precision is reached before the question is answered, then there should be a precision exception because we have approximation intervals that are not small enough to decide.

Laouen · 2019-04-15T20:26:48Z

include/real/real_algorithm.hpp

@@ -204,7 +204,9 @@ namespace boost {
             * @param exponent - an integer representing the number exponent.
             */
            explicit real_algorithm(int (*get_nth_digit)(unsigned int), int exponent)
-                    : _get_nth_digit(get_nth_digit), _exponent(exponent), _positive(true) {}
+                    : _get_nth_digit(get_nth_digit), _exponent(exponent), _positive(true) {
+                        set_maximum_precision(15);


Why is this max precision hardcoded with a 15? It should be the value of boost::real::real::maximum_precision which is the default max precision.

Suggested change

set_maximum_precision(15);

set_maximum_precision(boost::real::real::maximum_precision);

Why is this max precision hardcoded with a 15? It should be the value of boost::real::real::maximum_precision which is the default max precision.

Suggested change
set_maximum_precision(15);
set_maximum_precision(boost::real::real::maximum_precision);

maximum_precision is an uninitialised variable currently and seems to take the value 10. Also, that hardcoded 15 is for algorithm numbers. I don't know what to use as its precision so I just chose 15. Please suggest default max precision for algorithm numbers which might have infinite precision. Or what show max_precision be initialised with for algorithm numbers?

boost::real::real::maximum_precision and boost::real::real_algorithm::maximum_precision are global variables the user must set at the start of their code to specify the global maximum precision that is ever allowed. If you check the tests, you can notice in test/include/test_helpers.hpp in lines 9 and 10 that those variables are set in 10 and that is why they have that value (It is never hardcoded). With no exceptions, these variable is used as the max precision ever accepted and even for explicit numbers that have more digits than this maximum precision, they will throw a precision_exception.

The max precision is used to allow having some control in the algorithm memory and time complexities.

Oh no...
I never noticed the initialisations in test_helpers which has led to a lot of confusion for me as I couldn't figure out why the precision cap was at 10...
Now it seems like there wasn't any issue in the first place. The precision error I thought was a mistake was due to this max_precision of 10. And then the algorithm precision can just be kept as the max_precision set in test_helpers then.

Laouen · 2019-04-15T20:27:26Z

include/real/real_algorithm.hpp

@@ -224,7 +226,9 @@ namespace boost {
                                    bool positive)
                    : _get_nth_digit(get_nth_digit),
                      _exponent(exponent),
-                      _positive(positive) {}
+                      _positive(positive) {
+                          set_maximum_precision(15);


Suggested change

set_maximum_precision(15);

set_maximum_precision(boost::real::real::maximum_precision);

Laouen · 2019-04-15T20:32:45Z

test/real_algorithm_iterator_unit_test.cpp

@@ -149,7 +149,7 @@ TEST_CASE("Iterator cend") {

    SECTION("Iterate until the maximum set precision returns the end of the iterator") {

-        for(int i = 0; i < 9; i++) {
+        for(int i = 0; i < 14; i++) {


Suggested change

for(int i = 0; i < 14; i++) {

for(int i = 0; i < boost::real::real::maximum_precision-1; i++) {

Laouen · 2019-04-15T20:45:07Z

test/real_eq_operator_test.cpp

                boost::real::real b(one_one_one, 1);

-                CHECK_THROWS_AS(a == b, boost::real::precision_exception);
+                CHECK_FALSE(a == b);


This should throw a precision exception because b is an algorithmic number, the full precision of an algorithmic number is never reached (i.e b.lowe_bound() < b.upper_bound() is always true) and thus, you can never know if a == b.

Algorithmic numbers are thought as numbers with infinity digit size; b could be 1.110000000...........000001 and you will never know it because the max precision is reached before you get there. Even if you iterate enough to reach the last "1", there could be more non zero digits after that and you don't know until when you must iterate. This is known as an undecidable problem. You can now if a != b (if you iterate enough) but no if a == b.

Oh yeah, I originally thought one_one_one is actually 1.11111111. I'll make the change. This explains what I noticed and commented earlier in the code that 1.111 works but 1.11 doesn't. Cause b is 1.1100000...

Exactly, b = 1.11000...

Laouen · 2019-04-15T20:51:11Z

test/real_eq_operator_test.cpp

@@ -162,17 +162,17 @@ TEST_CASE("Operator ==") {

        SECTION("With precision exception") {


Read the section name "With precision exception" these tests should all through a precision exception.

There is a problem with your modifications, because algorithmic numbers can never be fully known and therefore, by definition of "fully known" (i.e. the upper and lower bounds of the approximation interval are equals) these numbers can never be compared with == and return a true. We are able to say that they are different ( != operator ) but not that they are equal ( == operator ).

The problem is that once you reach the maximum precision, you just asume you know the full number. For the numbers in the test, this happens because we constructed those numbers and we know that, but this is not something the Boost:Real library knows and the precision exception should be thrown.

Could I not just add a check for the EXPLICIT case? And for algorithm numbers, what precision to use for comparison as it can be arbitrarily high if not specified by set_precision

Laouen · 2019-04-15T21:02:54Z

test/real_eq_operator_test.cpp

+        }
+    }
+
+    SECTION("My Tests") {


The name can't be "My test". The name must be descriptive about what the section is testing. This is an open source code that more people will work on it and not a private code.

Sorry... I intended that as a temporary but forgot to remove it.

Laouen · 2019-04-15T21:05:42Z

test/real_greater_than_operator_test.cpp

                boost::real::real b(one_one_one, 1);

-                CHECK_THROWS_AS(a > b, boost::real::precision_exception);


The idea of the test is to have a precision error, that is why "a = 1.11" that is equal to "b", but because "b" is an algorithmic number, then, we can't know whether "a > b" or not.

If you change a to 1.111 you know that a is greater than b because you know that 1.111.... > 1.110.... for all the possible values of the remaining infinity lower digits of "b".

Yeah, same as before. I thought one_one_one is 1.111111...
Will fix

sdavtaker · 2019-05-24T14:32:05Z

Hi,
Can you resubmit this PR in https://github.com/BoostGSoC19/Real and close it here?
This Repository is frozen now that repository for 2019 exist.

SagnikDey92 closed this Apr 9, 2019

SagnikDey92 reopened this Apr 9, 2019

SagnikDey92 added 2 commits April 9, 2019 19:56

changed max_precision for real

58f46b8

fixed test files to pass travis

b228131

Laouen reviewed Apr 15, 2019

View reviewed changes

Laouen self-assigned this Apr 15, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

changed max_precision for real #18

changed max_precision for real #18

SagnikDey92 commented Apr 9, 2019 •

edited

Loading

Laouen left a comment

Laouen Apr 15, 2019

SagnikDey92 Apr 16, 2019

Laouen Apr 16, 2019

SagnikDey92 Apr 16, 2019

Laouen Apr 15, 2019

Laouen Apr 15, 2019

Laouen Apr 15, 2019

SagnikDey92 Apr 16, 2019

Laouen Apr 16, 2019

Laouen Apr 15, 2019

SagnikDey92 Apr 16, 2019

Laouen Apr 15, 2019

SagnikDey92 Apr 16, 2019

Laouen Apr 15, 2019

SagnikDey92 Apr 16, 2019

sdavtaker commented May 24, 2019

	set_maximum_precision(15);
	set_maximum_precision(boost::real::real::maximum_precision);

	for(int i = 0; i < 14; i++) {
	for(int i = 0; i < boost::real::real::maximum_precision-1; i++) {

		@@ -162,17 +162,17 @@ TEST_CASE("Operator ==") {

		SECTION("With precision exception") {

		boost::real::real b(one_one_one, 1);

		CHECK_THROWS_AS(a > b, boost::real::precision_exception);

changed max_precision for real #18

Are you sure you want to change the base?

changed max_precision for real #18

Conversation

SagnikDey92 commented Apr 9, 2019 • edited Loading

Laouen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sdavtaker commented May 24, 2019

SagnikDey92 commented Apr 9, 2019 •

edited

Loading