Moodle
  1. Moodle
  2. MDL-31837

Absolute tolerance is not calculated correctly when grading numerical, calculated and calculatedsimple questions

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.2, 2.2.4, 2.3.1, 2.4
    • Fix Version/s: 2.2.5, 2.3.2
    • Component/s: Questions
    • Labels:
    • Testing Instructions:
      Hide

      1.a Create a numerical question with tolerance 0, correct answer 0.

      1.b Check that 0 is graded right, and 1e-20 is graded wrong.

      2.a Create a numerical question with tolerance 1e-24, correct answer 0.

      2.b Check that 0 and 1e-24 are graded right and 1e-23 is graded wrong.

      3.a Create a numerical question with tolerance 0, correct answer 1e-20.

      3.b Check that 1e-20 and 1.0000000001e-20 are graded right, and 1.000001e-20 is graded wrong.

      4.a Create a numerical question with tolerance 1e-24, correct answer 1e-20.

      4.b Check that 1e-20 and 1.0001e-20 are graded right, and 1.0002e-20 is graded wrong.

      Show
      1.a Create a numerical question with tolerance 0, correct answer 0. 1.b Check that 0 is graded right, and 1e-20 is graded wrong. 2.a Create a numerical question with tolerance 1e-24, correct answer 0. 2.b Check that 0 and 1e-24 are graded right and 1e-23 is graded wrong. 3.a Create a numerical question with tolerance 0, correct answer 1e-20. 3.b Check that 1e-20 and 1.0000000001e-20 are graded right, and 1.000001e-20 is graded wrong. 4.a Create a numerical question with tolerance 1e-24, correct answer 1e-20. 4.b Check that 1e-20 and 1.0001e-20 are graded right, and 1.0002e-20 is graded wrong.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      38478

      Description

      When the answer tolerance is set to absolute type the calculation to take in account the PHP precision is not correctly done for small values.
      ex answer = 8.33e-24 tolerance 0 actual range value calculated -9.999999991664E-15 to 1.0000000008336E-14
      There was a small mismatch when transfering the calculations from the numerical/questiontype.php to new engine numerical/question.php

        Activity

        Hide
        Joshua Bragg added a comment -

        Pierre, any thoughts on when this might get fixed? I'm coming up on using those questions again in mid-September in my class.

        Show
        Joshua Bragg added a comment - Pierre, any thoughts on when this might get fixed? I'm coming up on using those questions again in mid-September in my class.
        Hide
        Pierre Pichet added a comment -

        Hi Joshua,
        I will come back to work mid august. So this could be fixed, if necessary, for your and my fall session as 8,33e-24 seems to be at the atomic scale...
        Before this, I will look at the problem to locate where the code has changed.

        Show
        Pierre Pichet added a comment - Hi Joshua, I will come back to work mid august. So this could be fixed, if necessary, for your and my fall session as 8,33e-24 seems to be at the atomic scale... Before this, I will look at the problem to locate where the code has changed.
        Hide
        Joshua Bragg added a comment -

        Good call on the guess. It was actually the energy of a photon.

        Thanks for responding on your vacation. I appreciate you working on this.

        Show
        Joshua Bragg added a comment - Good call on the guess. It was actually the energy of a photon. Thanks for responding on your vacation. I appreciate you working on this.
        Hide
        Pierre Pichet added a comment -

        Effectively the 1,9 does not account for th PHP precision limit when the tolerance is absolute not relative.
        Before moving back 2 coding to 1,9 , there is a need to an extended analysis of the various possibilities related to absolute and relative errors.
        The general moodle philosophy is to build the most flexible interface so that it satifies all teacher needs.
        However real life has shown that teachers could need some guidance

        As an example: Why do you use absolute 0 absolute tolerance i.e the student needs to give you the EXACT value with no more decimals than you set?
        He could use a hand calculator that is more precise than the Moodle PHP.

        Show
        Pierre Pichet added a comment - Effectively the 1,9 does not account for th PHP precision limit when the tolerance is absolute not relative. Before moving back 2 coding to 1,9 , there is a need to an extended analysis of the various possibilities related to absolute and relative errors. The general moodle philosophy is to build the most flexible interface so that it satifies all teacher needs. However real life has shown that teachers could need some guidance As an example: Why do you use absolute 0 absolute tolerance i.e the student needs to give you the EXACT value with no more decimals than you set? He could use a hand calculator that is more precise than the Moodle PHP.
        Hide
        Joshua Bragg added a comment -

        My students are using a hand calculator. This question (and several other similar ones) are part of quizzes designed to serve as an interactive homework check system.

        The general pattern for my class is that I give full credit for the correct answer with the correct significant figures and reduced credit for the correct answer with the wrong significant figures.

        However, that 0 tolerance isn't the real problem here. Even if the tolerance is not 0 the error still appears. If I change it to 8.33E-24 with tolerance of 2E-24 and put in the answer as 4E-24 which is outside the tolerance range, it will still mark it correct.

        Show
        Joshua Bragg added a comment - My students are using a hand calculator. This question (and several other similar ones) are part of quizzes designed to serve as an interactive homework check system. The general pattern for my class is that I give full credit for the correct answer with the correct significant figures and reduced credit for the correct answer with the wrong significant figures. However, that 0 tolerance isn't the real problem here. Even if the tolerance is not 0 the error still appears. If I change it to 8.33E-24 with tolerance of 2E-24 and put in the answer as 4E-24 which is outside the tolerance range, it will still mark it correct.
        Hide
        Pierre Pichet added a comment -

        Your point is right.
        I will take a look at the code although on my ipad I cannot experiment.

        Show
        Pierre Pichet added a comment - Your point is right. I will take a look at the code although on my ipad I cannot experiment.
        Hide
        Pierre Pichet added a comment -

        I pinpoint the problem and put this on my todo list on return from vacancy.

        Show
        Pierre Pichet added a comment - I pinpoint the problem and put this on my todo list on return from vacancy.
        Hide
        Pierre Pichet added a comment -

        Tim,
        In numerical/ question.php the tolerance for nominal for a small answer (i.e. 1e-20) should not use the max function as the tiny fraction will be calculated with 1 and not the answer (ex 1e-20).
        This is the way the function was used in 1,9.

        The ini_get('precision') is the limit of the digit part of the number, its real value is related to the exponent part of the number. This is why it is multiply by the answer value in the original code.

         320             throw new coding_exception('Cannot work out tolerance interval for answer *.');
         321         }
         322 
         323         // We need to add a tiny fraction depending on the set precision to make
         324         // the comparison work correctly, otherwise seemingly equal values can
         325         // yield false. See MDL-3225.
         326         $tolerance = (float) $this->tolerance + pow(10, -1 * ini_get('precision'));
         327 
         328         switch ($this->tolerancetype) {
         329             case 1: case 'relative':
         330                 $range = abs($this->answer) * $tolerance;
         331                 return array($this->answer - $range, $this->answer + $range);
         332 
         333             case 2: case 'nominal':
         334                 $tolerance = $this->tolerance + pow(10, -1 * ini_get('precision')) *
         335                         max(1, abs($this->answer));
         336                 return array($this->answer - $tolerance, $this->answer + $tolerance);
         337 
         338             case 3: case 'geometric':
         339                 $quotient = 1 + abs($tolerance);
         340                 return array($this->answer / $quotient, $this->answer * $quotient);
         341 
         342             default:
         343                 throw new coding_exception('Unknown tolerance type ' . $this->tolerancetype);
         344         }
         345     }
         346 
         
        
        
        Show
        Pierre Pichet added a comment - Tim, In numerical/ question.php the tolerance for nominal for a small answer (i.e. 1e-20) should not use the max function as the tiny fraction will be calculated with 1 and not the answer (ex 1e-20). This is the way the function was used in 1,9. The ini_get('precision') is the limit of the digit part of the number, its real value is related to the exponent part of the number. This is why it is multiply by the answer value in the original code. 320             throw new coding_exception('Cannot work out tolerance interval for answer *.'); 321         } 322 323         // We need to add a tiny fraction depending on the set precision to make 324         // the comparison work correctly, otherwise seemingly equal values can 325         // yield false. See MDL-3225. 326         $tolerance = (float) $this->tolerance + pow(10, -1 * ini_get('precision')); 327 328         switch ($this->tolerancetype) { 329             case 1: case 'relative': 330                 $range = abs($this->answer) * $tolerance; 331                 return array($this->answer - $range, $this->answer + $range); 332 333             case 2: case 'nominal': 334                 $tolerance = $this->tolerance + pow(10, -1 * ini_get('precision')) * 335                         max(1, abs($this->answer)); 336                 return array($this->answer - $tolerance, $this->answer + $tolerance); 337 338             case 3: case 'geometric': 339                 $quotient = 1 + abs($tolerance); 340                 return array($this->answer / $quotient, $this->answer * $quotient); 341 342             default: 343                 throw new coding_exception('Unknown tolerance type ' . $this->tolerancetype); 344         } 345     } 346
        Hide
        Tim Hunt added a comment -

        Yes, so the max(1, ...) is wrong. However, we need to deal correctly with the case where $this->answer is 0.

        Show
        Tim Hunt added a comment - Yes, so the max(1, ...) is wrong. However, we need to deal correctly with the case where $this->answer is 0.
        Hide
        Pierre Pichet added a comment -

        I Will do some tests on m'y iPad ... and give the results.

        Show
        Pierre Pichet added a comment - I Will do some tests on m'y iPad ... and give the results.
        Hide
        Pierre Pichet added a comment -

        Tim,
        I think we sould consider 0 a a special case as 0 is quite a special number.
        Either 0 is an integer as could be the case in numerical question or a quite specific result of a calculation in calculated questions.
        Either the tolerance to a 0 answer is 0 i.e. nothing else or 0 within the calculated precision (i.e. amount of money )
        Using the tolerance parameter the user can define exactly what he wants or tolerates to the 0 answer.
        The additional factor related to the PHP precision does not work for 0 PHP internal representation as for other numbers where the digital digits have different values related to the answer value.
        Using the common formula for other answer values will give 0 to the tiny fraction a 0 value for answer== 0 and only the tolerance defined by the user (teacher) will be applied.
        As an example on money calculations the user can define the tolerance as 0,00 i.e. cents.
        Perhaps you could set a forum discussion on this if you think it's necessary.

        Show
        Pierre Pichet added a comment - Tim, I think we sould consider 0 a a special case as 0 is quite a special number. Either 0 is an integer as could be the case in numerical question or a quite specific result of a calculation in calculated questions. Either the tolerance to a 0 answer is 0 i.e. nothing else or 0 within the calculated precision (i.e. amount of money ) Using the tolerance parameter the user can define exactly what he wants or tolerates to the 0 answer. The additional factor related to the PHP precision does not work for 0 PHP internal representation as for other numbers where the digital digits have different values related to the answer value. Using the common formula for other answer values will give 0 to the tiny fraction a 0 value for answer== 0 and only the tolerance defined by the user (teacher) will be applied. As an example on money calculations the user can define the tolerance as 0,00 i.e. cents. Perhaps you could set a forum discussion on this if you think it's necessary.
        Hide
        Tim Hunt added a comment -

        I agree that 0 is a special case.

        I don't currently have strong opinions about the 'right' way to handle it.

        I think we should be able to find a way to handle it using the existing options on the question editing form that just works.

        Show
        Tim Hunt added a comment - I agree that 0 is a special case. I don't currently have strong opinions about the 'right' way to handle it. I think we should be able to find a way to handle it using the existing options on the question editing form that just works.
        Hide
        Pierre Pichet added a comment -

        I will experiment the various combinations and use docs to help clarify the case.

        Show
        Pierre Pichet added a comment - I will experiment the various combinations and use docs to help clarify the case.
        Hide
        Pierre Pichet added a comment -

        Tim,
        I put a first version of my analysis on the docs.

        http://docs.moodle.org/dev/Question_Engine_2:Numerical_tolerances

        Show
        Pierre Pichet added a comment - Tim, I put a first version of my analysis on the docs. http://docs.moodle.org/dev/Question_Engine_2:Numerical_tolerances
        Hide
        Pierre Pichet added a comment -

        There is only a very little discrepancy between my actual proposal on the docs and the 1,9 version in 0 answer case that could result from a calculation formula.
        This discrepancy is perhaps just apparent as it implies the internal PHP rounding code for 0 result.
        However the 1,9 code which inherits from preceeding versions, has been thoroughly tested.

        I think we should first put a 1,9 equivalent code back and create the tests later.( perhaps create a separate issue)
        There is a lot of 1,9 -> 2,3 migration for next fall session that could justify to do it this way.
        In any case, given the actual discussion, this code will not be redone in a near future...

        Show
        Pierre Pichet added a comment - There is only a very little discrepancy between my actual proposal on the docs and the 1,9 version in 0 answer case that could result from a calculation formula. This discrepancy is perhaps just apparent as it implies the internal PHP rounding code for 0 result. However the 1,9 code which inherits from preceeding versions, has been thoroughly tested. I think we should first put a 1,9 equivalent code back and create the tests later.( perhaps create a separate issue) There is a lot of 1,9 -> 2,3 migration for next fall session that could justify to do it this way. In any case, given the actual discussion, this code will not be redone in a near future...
        Hide
        Tim Hunt added a comment -

        Thank you for the excellent comparison. http://docs.moodle.org/dev/Question_Engine_2:Numerical_tolerances#Tests_results makes it very easy to compare the various implementations.

        I have to say, however, I think Tim's proposal is the best. Look at the top row, and compare it to the second row. I think that the interval for tolerance 0 should be smaller than the interval for tolerance 1e-24. In all the other cases your proposal and mine are both very close to the 1.9 implementation, which is definitely better than the 2.0 code.

        Show
        Tim Hunt added a comment - Thank you for the excellent comparison. http://docs.moodle.org/dev/Question_Engine_2:Numerical_tolerances#Tests_results makes it very easy to compare the various implementations. I have to say, however, I think Tim's proposal is the best. Look at the top row, and compare it to the second row. I think that the interval for tolerance 0 should be smaller than the interval for tolerance 1e-24. In all the other cases your proposal and mine are both very close to the 1.9 implementation, which is definitely better than the 2.0 code.
        Hide
        Pierre Pichet added a comment -

        Effectively it should be lower.
        However when applied to 0.0, I think that the PHP code is working with a rounding that use the $epsilon i.e. 1e-14 as the 0.O use all the mantissa to define itself as different from any lower value to the 1e-300 limit.
        Say differently 0 - 1e-24 is not different than 0 +1e-24 when applied to 0.0 so you are not testing a real MIN MAX.

        I will do some tests but first the breakfast

        Show
        Pierre Pichet added a comment - Effectively it should be lower. However when applied to 0.0, I think that the PHP code is working with a rounding that use the $epsilon i.e. 1e-14 as the 0.O use all the mantissa to define itself as different from any lower value to the 1e-300 limit. Say differently 0 - 1e-24 is not different than 0 +1e-24 when applied to 0.0 so you are not testing a real MIN MAX. I will do some tests but first the breakfast
        Hide
        Joshua Bragg added a comment -

        Tim and Pierre, thanks for all your hard work on this. I keep looking at all this stuff with great interest. I wish I could help... I appreciate the thought and effort on this.

        Show
        Joshua Bragg added a comment - Tim and Pierre, thanks for all your hard work on this. I keep looking at all this stuff with great interest. I wish I could help... I appreciate the thought and effort on this.
        Hide
        Pierre Pichet added a comment -

        Tim,
        The PHP rounding for 0 as answer is different following the way the result is obtained.

        http://docs.moodle.org/dev/Question_Engine_2:Numerical_tolerances#HOW_PHP_handle_0

        There seems to be no clear way to handle the 0 case.
        The solution should follow the policy behind the epsilon calculation of grading the answer without conflict from the students.

        Show
        Pierre Pichet added a comment - Tim, The PHP rounding for 0 as answer is different following the way the result is obtained. http://docs.moodle.org/dev/Question_Engine_2:Numerical_tolerances#HOW_PHP_handle_0 There seems to be no clear way to handle the 0 case. The solution should follow the policy behind the epsilon calculation of grading the answer without conflict from the students.
        Hide
        Tim Hunt added a comment -

        It don't understand the point you are trying to make there.

        I cannot imagine any real question where we would want the student to compute 1-1.0000000000000001 or 1-1.000000000000001. If you are interested in those sorts of numbers, then double precision is not good enough.

        In any calculated question, we expect the teacher to set a sensible precision themselves, which is likely to involve only a few significant figures or decimal places.

        There was a real problem with answers around 10^-20, which we now have a proposed solution for. Unless I am missing something, I think that is good enough for all practical purposes.

        Show
        Tim Hunt added a comment - It don't understand the point you are trying to make there. I cannot imagine any real question where we would want the student to compute 1-1.0000000000000001 or 1-1.000000000000001. If you are interested in those sorts of numbers, then double precision is not good enough. In any calculated question, we expect the teacher to set a sensible precision themselves, which is likely to involve only a few significant figures or decimal places. There was a real problem with answers around 10^-20, which we now have a proposed solution for. Unless I am missing something, I think that is good enough for all practical purposes.
        Hide
        Pierre Pichet added a comment - - edited

        I can imagine the case (pH calculation) where the calculated answer is 0, even more simply in money problems involving the EURO debts calculated to cents involving a precision greater than the $epsilon.
        I imagine with more difficulties that the teacher set a 0 tolerance for such an answer.

        The case of answer == 0 with a nominal interval of 0 where the 0 as answer result from a formula involving high precision
        is effectively more theoretical than real.

        I put the comment on the docs to cover all aspects as this could be useful in the future.

        I agree with your solution ( Tim's idea ) and given my limited disponibilities in the next weeks (courses are back after a 5 months student strike and I need to install the PHP testing ), perhaps you should continue on this bug.

        Show
        Pierre Pichet added a comment - - edited I can imagine the case (pH calculation) where the calculated answer is 0, even more simply in money problems involving the EURO debts calculated to cents involving a precision greater than the $epsilon. I imagine with more difficulties that the teacher set a 0 tolerance for such an answer. The case of answer == 0 with a nominal interval of 0 where the 0 as answer result from a formula involving high precision is effectively more theoretical than real. I put the comment on the docs to cover all aspects as this could be useful in the future. I agree with your solution ( Tim's idea ) and given my limited disponibilities in the next weeks (courses are back after a 5 months student strike and I need to install the PHP testing ), perhaps you should continue on this bug.
        Hide
        Pierre Pichet added a comment - - edited

        Your question on docs
        " In particular, what tests would you like to add that will fail with the current code, but which will pass once we have fixed this bug?
        "
        This code is used by numerical, calculated and calculatedsimple questiontypes, the last 2 questiontypes using all the diferent tolerance types.
        The tests should create the different conditions related to the actual code structure and verify that the min and max values are the ones expected.
        This is not evident as these min, max values are calculated using the precision limit.

        However giving that the tests need to be built following the code flow, I think that they are not necessary if we just put a warning in the code to not change this code flow until the tests as done on the docs remain valid.

        P.S. There was already such a warning in the 1,9 code i.e."See MDL-3225.", so perhaps my proposal is not the thing todo

        Show
        Pierre Pichet added a comment - - edited Your question on docs " In particular, what tests would you like to add that will fail with the current code, but which will pass once we have fixed this bug? " This code is used by numerical, calculated and calculatedsimple questiontypes, the last 2 questiontypes using all the diferent tolerance types. The tests should create the different conditions related to the actual code structure and verify that the min and max values are the ones expected. This is not evident as these min, max values are calculated using the precision limit. However giving that the tests need to be built following the code flow, I think that they are not necessary if we just put a warning in the code to not change this code flow until the tests as done on the docs remain valid. P.S. There was already such a warning in the 1,9 code i.e."See MDL-3225 .", so perhaps my proposal is not the thing todo
        Hide
        Tim Hunt added a comment -

        Thanks to your excellent investigation, it will now be easy to make a patch and submit it for integration, so I will do that.

        I think the test-cases to add are basically the ones you put in the table on the wiki, so I will use the examples there.

        Show
        Tim Hunt added a comment - Thanks to your excellent investigation, it will now be easy to make a patch and submit it for integration, so I will do that. I think the test-cases to add are basically the ones you put in the table on the wiki, so I will use the examples there.
        Hide
        Tim Hunt added a comment -

        Patch made based on Pierre's research, and submitted for integration.

        Show
        Tim Hunt added a comment - Patch made based on Pierre's research, and submitted for integration.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

        TIA and ciao

        Show
        Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
        Hide
        Aparup Banerjee added a comment -

        Thanks, thats been integrated into 22 (no test), 23 and master.

        curious: not adding any docs_required tag as this is a correction, but i am wondering if there are docs describing these intricate calculations as an overview. (disclaimer: lazy, not searched)

        Show
        Aparup Banerjee added a comment - Thanks, thats been integrated into 22 (no test), 23 and master. curious: not adding any docs_required tag as this is a correction, but i am wondering if there are docs describing these intricate calculations as an overview. (disclaimer: lazy, not searched)
        Hide
        Aparup Banerjee added a comment -

        ah this is creating warnings for 22 :
        E_USER_WARNING: unexpected operator '+' in /Users/Shared/Jenkins/Home/git_repositories/MOODLE_22_STABLE/lib/evalmath/evalmath.class.php on line 440
        E_USER_WARNING: expecting a closing bracket in /Users/Shared/Jenkins/Home/git_repositories/MOODLE_22_STABLE/lib/evalmath/evalmath.class.php on line 440
        E_USER_WARNING: operator '^' lacks operand in /Users/Shared/Jenkins/Home/git_repositories/MOODLE_22_STABLE/lib/evalmath/evalmath.class.php on line 440

        i'll need to sink deeper into this to understand them.. no idea why atm.

        Show
        Aparup Banerjee added a comment - ah this is creating warnings for 22 : E_USER_WARNING: unexpected operator '+' in /Users/Shared/Jenkins/Home/git_repositories/MOODLE_22_STABLE/lib/evalmath/evalmath.class.php on line 440 E_USER_WARNING: expecting a closing bracket in /Users/Shared/Jenkins/Home/git_repositories/MOODLE_22_STABLE/lib/evalmath/evalmath.class.php on line 440 E_USER_WARNING: operator '^' lacks operand in /Users/Shared/Jenkins/Home/git_repositories/MOODLE_22_STABLE/lib/evalmath/evalmath.class.php on line 440 i'll need to sink deeper into this to understand them.. no idea why atm.
        Hide
        Tim Hunt added a comment -

        It is more likely that the problem is with the test question you made.

        The Calculated question type evaluates equations typed in by the teacher. Those error messages look like an error in what the tester typed, not in the Moodle code.

        Show
        Tim Hunt added a comment - It is more likely that the problem is with the test question you made. The Calculated question type evaluates equations typed in by the teacher. Those error messages look like an error in what the tester typed, not in the Moodle code.
        Hide
        Pierre Pichet added a comment -

        I agree with Tim.
        Even if the code that analyze the formula is an old one, it is not handling perfectly all the users errors. MDL-26823 is an example of this.
        I could supply some specific examples if necessary.

        Show
        Pierre Pichet added a comment - I agree with Tim. Even if the code that analyze the formula is an old one, it is not handling perfectly all the users errors. MDL-26823 is an example of this. I could supply some specific examples if necessary.
        Hide
        Aparup Banerjee added a comment -

        ah ok, i take it this won't cause bc problems then. all's well

        Show
        Aparup Banerjee added a comment - ah ok, i take it this won't cause bc problems then. all's well
        Hide
        Aparup Banerjee added a comment - - edited

        /question/type/numerical/simpletest/testanswer.php line 49 in 22 needs to be updated - cause of failure for simpletest.

        ps: please disregard previous comment's user warnings - they were totally irrelevant

        Show
        Aparup Banerjee added a comment - - edited /question/type/numerical/simpletest/testanswer.php line 49 in 22 needs to be updated - cause of failure for simpletest. ps: please disregard previous comment's user warnings - they were totally irrelevant
        Hide
        Eloy Lafuente (stronk7) added a comment -

        somebody missed to backport tests changes to 22_STABLE yup, so the old test (now deleted in 23 and master) is breaking 22 simpletests.

        Show
        Eloy Lafuente (stronk7) added a comment - somebody missed to backport tests changes to 22_STABLE yup, so the old test (now deleted in 23 and master) is breaking 22 simpletests.
        Hide
        Tim Hunt added a comment -

        Oh, Grrr! I will amend the 22_stable commit. Sorry for missing that.

        Show
        Tim Hunt added a comment - Oh, Grrr! I will amend the 22_stable commit. Sorry for missing that.
        Hide
        Tim Hunt added a comment -
        Show
        Tim Hunt added a comment - OK, new commit added to https://github.com/timhunt/moodle/compare/MOODLE_22_STABLE...MDL-31837_22
        Hide
        Pierre Pichet added a comment -

        Tim,
        When you set your test values as in

           // Zero tol, non-zero answer.
                $answer = new qtype_numerical_answer(13, 1e-20, 1.0, '', FORMAT_MOODLE, 0.0);
                $this->assertFalse($answer->within_tolerance(0.9999999e-20));
                $this->assertTrue($answer->within_tolerance(1e-20));
                $this->assertFalse($answer->within_tolerance(1.0000001e-20));
        
                // Non-zero tol, zero answer.
                $answer = new qtype_numerical_answer(13, 0.0, 1.0, '', FORMAT_MOODLE, 1e-24);
                $this->assertFalse($answer->within_tolerance(-2e-24));
                $this->assertTrue($answer->within_tolerance(-1e-24));
                $this->assertTrue($answer->within_tolerance(0));
                $this->assertTrue($answer->within_tolerance(1e-24));
                $this->assertFalse($answer->within_tolerance(2e-24));
        
                // Zero tol, zero answer.
                $answer = new qtype_numerical_answer(13, 0.0, 1.0, '', FORMAT_MOODLE, 1e-24);
                $this->assertFalse($answer->within_tolerance(-1e-20));
                $this->assertTrue($answer->within_tolerance(-1e-35));
                $this->assertTrue($answer->within_tolerance(0));
                $this->assertTrue($answer->within_tolerance(1e-35));
                $this->assertFalse($answer->within_tolerance(1e-20));
        

        the values should use epsilon so that they will fit in any PHP settings.

        Show
        Pierre Pichet added a comment - Tim, When you set your test values as in // Zero tol, non-zero answer. $answer = new qtype_numerical_answer(13, 1e-20, 1.0, '', FORMAT_MOODLE, 0.0); $ this ->assertFalse($answer->within_tolerance(0.9999999e-20)); $ this ->assertTrue($answer->within_tolerance(1e-20)); $ this ->assertFalse($answer->within_tolerance(1.0000001e-20)); // Non-zero tol, zero answer. $answer = new qtype_numerical_answer(13, 0.0, 1.0, '', FORMAT_MOODLE, 1e-24); $ this ->assertFalse($answer->within_tolerance(-2e-24)); $ this ->assertTrue($answer->within_tolerance(-1e-24)); $ this ->assertTrue($answer->within_tolerance(0)); $ this ->assertTrue($answer->within_tolerance(1e-24)); $ this ->assertFalse($answer->within_tolerance(2e-24)); // Zero tol, zero answer. $answer = new qtype_numerical_answer(13, 0.0, 1.0, '', FORMAT_MOODLE, 1e-24); $ this ->assertFalse($answer->within_tolerance(-1e-20)); $ this ->assertTrue($answer->within_tolerance(-1e-35)); $ this ->assertTrue($answer->within_tolerance(0)); $ this ->assertTrue($answer->within_tolerance(1e-35)); $ this ->assertFalse($answer->within_tolerance(1e-20)); the values should use epsilon so that they will fit in any PHP settings.
        Hide
        Tim Hunt added a comment -

        I think I disagree. I do not believe that PHP epsilon will ever be different from 1e-14.

        If that belief is wrong, I would like to know about it. Therefore, having some unit tests that will fail if the assumption is wrong seems like a good idea to me.

        Show
        Tim Hunt added a comment - I think I disagree. I do not believe that PHP epsilon will ever be different from 1e-14. If that belief is wrong, I would like to know about it. Therefore, having some unit tests that will fail if the assumption is wrong seems like a good idea to me.
        Hide
        Pierre Pichet added a comment -

        You are right as long as the numbers are using the "standard" IEE values.

        from http://www.php.net/manual/en/language.types.float.php

        The size of a float is platform-dependent, although a maximum of ~1.8e308 with a
        precision of roughly 14 decimal digits is a common value (the 64 bit IEEE format).

        also http://en.wikipedia.org/wiki/Double-precision_floating-point_format

        So your "wait and see" policy is a valid option.

        Show
        Pierre Pichet added a comment - You are right as long as the numbers are using the "standard" IEE values. from http://www.php.net/manual/en/language.types.float.php The size of a float is platform-dependent, although a maximum of ~1.8e308 with a precision of roughly 14 decimal digits is a common value (the 64 bit IEEE format). also http://en.wikipedia.org/wiki/Double-precision_floating-point_format So your "wait and see" policy is a valid option.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        The MOODLE_22_STABLE extra commit has been integrated, leading to passed simpletests, thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - The MOODLE_22_STABLE extra commit has been integrated, leading to passed simpletests, thanks!
        Hide
        Aparup Banerjee added a comment -

        Thanks Eloy. just noting here that 2a and 2b work fine here (for Raj)

        Show
        Aparup Banerjee added a comment - Thanks Eloy. just noting here that 2a and 2b work fine here (for Raj)
        Hide
        Rajesh Taneja added a comment -

        Works Great
        Thanks for fixing this Tim.

        FYI: 3.b Values seems wrong, although tested with 1e-5 => 0.00001 and works fine. Hope this is what is expected.

        Show
        Rajesh Taneja added a comment - Works Great Thanks for fixing this Tim. FYI: 3.b Values seems wrong, although tested with 1e-5 => 0.00001 and works fine. Hope this is what is expected.
        Hide
        Tim Hunt added a comment -

        Which value seems wrong in 3.b?

        Show
        Tim Hunt added a comment - Which value seems wrong in 3.b?
        Hide
        Rajesh Taneja added a comment -

        Check that 1e-20 and 1.0000000001 are graded right (When tolerance is 0)

        Show
        Rajesh Taneja added a comment - Check that 1e-20 and 1.0000000001 are graded right (When tolerance is 0)
        Hide
        Tim Hunt added a comment -

        Oops yes. That should have been 1.0000000001e-20. Thanks.

        Show
        Tim Hunt added a comment - Oops yes. That should have been 1.0000000001e-20. Thanks.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        I'm so proud...of you, many thanks!

        http://youtu.be/n64CdfDRnZY

        Closing as fixed, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - I'm so proud...of you, many thanks! http://youtu.be/n64CdfDRnZY Closing as fixed, ciao

          People

          • Votes:
            1 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: