Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-31837

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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:

      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

        Gliffy Diagrams

          Activity

          Hide
          worfds91 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
          worfds91 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
          ppichet 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
          ppichet 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
          worfds91 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
          worfds91 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
          ppichet 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
          ppichet 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
          worfds91 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
          worfds91 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
          ppichet 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
          ppichet 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
          ppichet Pierre Pichet added a comment -

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

          Show
          ppichet Pierre Pichet added a comment - I pinpoint the problem and put this on my todo list on return from vacancy.
          Hide
          ppichet 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
          ppichet 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
          timhunt 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
          timhunt 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
          ppichet Pierre Pichet added a comment -

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

          Show
          ppichet Pierre Pichet added a comment - I Will do some tests on m'y iPad ... and give the results.
          Hide
          ppichet 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
          ppichet 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
          timhunt 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
          timhunt 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
          ppichet Pierre Pichet added a comment -

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

          Show
          ppichet Pierre Pichet added a comment - I will experiment the various combinations and use docs to help clarify the case.
          Hide
          ppichet 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
          ppichet 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
          ppichet 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
          ppichet 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
          timhunt 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
          timhunt 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
          ppichet 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
          ppichet 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
          worfds91 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
          worfds91 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
          ppichet 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
          ppichet 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
          timhunt 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
          timhunt 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
          ppichet 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
          ppichet 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
          ppichet 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
          ppichet 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
          timhunt 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
          timhunt 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
          timhunt Tim Hunt added a comment -

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

          Show
          timhunt Tim Hunt added a comment - Patch made based on Pierre's research, and submitted for integration.
          Hide
          stronk7 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
          stronk7 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
          nebgor 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
          nebgor 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
          nebgor 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
          nebgor 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
          timhunt 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
          timhunt 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
          ppichet 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
          ppichet 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
          nebgor Aparup Banerjee added a comment -

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

          Show
          nebgor Aparup Banerjee added a comment - ah ok, i take it this won't cause bc problems then. all's well
          Hide
          nebgor 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
          nebgor 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
          stronk7 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
          stronk7 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
          timhunt Tim Hunt added a comment -

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

          Show
          timhunt Tim Hunt added a comment - Oh, Grrr! I will amend the 22_stable commit. Sorry for missing that.
          Hide
          timhunt Tim Hunt added a comment -
          Show
          timhunt Tim Hunt added a comment - OK, new commit added to https://github.com/timhunt/moodle/compare/MOODLE_22_STABLE...MDL-31837_22
          Hide
          ppichet 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
          ppichet 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
          timhunt 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
          timhunt 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
          ppichet 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
          ppichet 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
          stronk7 Eloy Lafuente (stronk7) added a comment -

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

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

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

          Show
          nebgor Aparup Banerjee added a comment - Thanks Eloy. just noting here that 2a and 2b work fine here (for Raj)
          Hide
          rajeshtaneja 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
          rajeshtaneja 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
          timhunt Tim Hunt added a comment -

          Which value seems wrong in 3.b?

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

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

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

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

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

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

          http://youtu.be/n64CdfDRnZY

          Closing as fixed, ciao

          Show
          stronk7 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:
                Fix Release Date:
                10/Sep/12