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

Edge case missing from Calculated qtype

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.4, 2.1.1, 2.2
    • Fix Version/s: 2.0.5, 2.1.2
    • Component/s: Questions
    • Labels:
    • Testing Instructions:
      Hide

      1. Start creating a new calculated question with all defaults, except:
      Name: <whatever>
      Answer Formula:

      {x}

      *1E-12
      Answer Grade: 100%
      Format: Significant Figures

      2. Click Save changes then Next page

      3. On this page set
      Range of Values: 1.0-1.1
      Decimal places: 1
      Then Add 30 new set(s) of wild card(s) values

      4. Verify that there are no errors like "ERROR Correct answer : 100e-13 outside limits of true value 1.0E-12"

      Show
      1. Start creating a new calculated question with all defaults, except: Name: <whatever> Answer Formula: {x} *1E-12 Answer Grade: 100% Format: Significant Figures 2. Click Save changes then Next page 3. On this page set Range of Values: 1.0-1.1 Decimal places: 1 Then Add 30 new set(s) of wild card(s) values 4. Verify that there are no errors like "ERROR Correct answer : 100e-13 outside limits of true value 1.0E-12"
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      When using the significant figures calculation mode, an edge case is not properly handled in the function 'qtype_calculated_calculate_answer', causing the calculated answer to be incorrect.

      The function works to ensure the answer is expressed as a string of the format "0.[1-9][0-9]*", but subsequently calls the round() function. In some cases, round will round up to 1.0- which is no longer in the assumed format. This causes an answer to be produced which is exactly 10x larger than the correct answer.

      A special case should be added to compensate, as in the attached patch.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            timhunt Tim Hunt added a comment -

            Would be nice to see some additional unit tests to cover this case.

            Show
            timhunt Tim Hunt added a comment - Would be nice to see some additional unit tests to cover this case.
            Hide
            ppichet Pierre Pichet added a comment -

            Kim,
            Have you a numerical example that we could use to test.
            However these ronding effects are also related to the PHP precision settings.

            Show
            ppichet Pierre Pichet added a comment - Kim, Have you a numerical example that we could use to test. However these ronding effects are also related to the PHP precision settings.
            Hide
            ktemkin Kyle Temkin added a comment -

            Try creating the following Simple Calculated Question:

            All defaults, except:
            Name: <whatever>
            Answer Formula:

            {x}E-12
            Answer Grade: 100%
            Format: Significant Figures

            Wildcard range for {x}

            : 1.0-1.1
            Decimal places: 1

            Then, generate any large amount of wildcards. I chose 30. I receive the following error on any wildcard generated with the value 1.0:

            "ERROR Correct answer : 100e-13 outside limits of true value 1.0E-12"

            Show
            ktemkin Kyle Temkin added a comment - Try creating the following Simple Calculated Question: All defaults, except: Name: <whatever> Answer Formula: {x}E-12 Answer Grade: 100% Format: Significant Figures Wildcard range for {x} : 1.0-1.1 Decimal places: 1 Then, generate any large amount of wildcards. I chose 30. I receive the following error on any wildcard generated with the value 1.0: "ERROR Correct answer : 100e-13 outside limits of true value 1.0E-12"
            Hide
            ktemkin Kyle Temkin added a comment -

            The primary problem is a rounding issue- which is more common than you'd think, given the way floating point numbers are stored internally.

            Here's the problem described and demonstrated in a simple PHP script:

            http://labs.ktemkin.com/moodle/bugs/rounding.phps

            You can execute it on my server here:
            http://labs.ktemkin.com/moodle/bugs/rounding.php

            And with the proposed patch:

            http://labs.ktemkin.com/moodle/bugs/rounding_patched.phps
            http://labs.ktemkin.com/moodle/bugs/rounding_patched.php

            Let me know if you have any questions.

            Show
            ktemkin Kyle Temkin added a comment - The primary problem is a rounding issue- which is more common than you'd think, given the way floating point numbers are stored internally. Here's the problem described and demonstrated in a simple PHP script: http://labs.ktemkin.com/moodle/bugs/rounding.phps You can execute it on my server here: http://labs.ktemkin.com/moodle/bugs/rounding.php And with the proposed patch: http://labs.ktemkin.com/moodle/bugs/rounding_patched.phps http://labs.ktemkin.com/moodle/bugs/rounding_patched.php Let me know if you have any questions.
            Hide
            ppichet Pierre Pichet added a comment -

            Thanks Kim,

            Things are ever more complex as PHP being an interpreted language that can tolerate such an expression as

            {x}E-12.

            look at the following case


             {x}

            E-12 1.0E-12 = 10000e-13
            Min: 9.8999999999999E-13---Max: 1.01E-12
            Erreur de réponse correcte : 10000e-13 en dehors des limites de la valeur réelle 1.0E-12
             
            This was from UQAM (my university) 1,9 site with calculated question.
            This part of the code can be trace back to older version.

            How do I got 10000e-13.

            This was by setting the answer with 4 significant figures.

            As long as the x is 1.0 (i.e. 1.000 ) all the expression is not correctly handled.
            If the value is different as 1.00001 things are OK.

            The only computer I have access (holidays...) is an Ipad .

            Could you test if your patch applied to moodle code also corrected this bad rounding effect.
             

            Show
            ppichet Pierre Pichet added a comment - Thanks Kim, Things are ever more complex as PHP being an interpreted language that can tolerate such an expression as {x}E-12. look at the following case  {x} E-12 1.0E-12 = 10000e-13 Min: 9.8999999999999E-13---Max: 1.01E-12 Erreur de réponse correcte : 10000e-13 en dehors des limites de la valeur réelle 1.0E-12   This was from UQAM (my university) 1,9 site with calculated question. This part of the code can be trace back to older version. How do I got 10000e-13. This was by setting the answer with 4 significant figures. As long as the x is 1.0 (i.e. 1.000 ) all the expression is not correctly handled. If the value is different as 1.00001 things are OK. The only computer I have access (holidays...) is an Ipad . Could you test if your patch applied to moodle code also corrected this bad rounding effect.  
            Hide
            ppichet Pierre Pichet added a comment -

            As a reminder, I also noticed that the left unit option is not handled correctly.

            if (1 == $answerlength)

            { $calculated->answer = $sign.$answer.$exponent.$unit; }

            else {
            // Attach additional zeros at the end of $answer,

            Show
            ppichet Pierre Pichet added a comment - As a reminder, I also noticed that the left unit option is not handled correctly. if (1 == $answerlength) { $calculated->answer = $sign.$answer.$exponent.$unit; } else { // Attach additional zeros at the end of $answer,
            Hide
            timhunt Tim Hunt added a comment -

            Pierre, I think Kyle's fix is good, so submitting it for integration.

            You are right that unit handling is wrong here, but I think we should handle that as a separate issue.

            By the way, a formula like

            {x}E-12 does not work when it comes to attempting the question. It needs to be written as {x}

            *1E-12, which seems far more sensible to me anyway.

            Show
            timhunt Tim Hunt added a comment - Pierre, I think Kyle's fix is good, so submitting it for integration. You are right that unit handling is wrong here, but I think we should handle that as a separate issue. By the way, a formula like {x}E-12 does not work when it comes to attempting the question. It needs to be written as {x} *1E-12, which seems far more sensible to me anyway.
            Hide
            ppichet Pierre Pichet added a comment -

            By the way, a formula like

            {x}E-12 COULD work when it comes to attempting the question.
            The {x}

            will be converted to the numerical to which the exponent will be applied as E number style is allowed in PHP
            2.4E3 = 2400
            I use this here to test the conversion process as any operation (ie.

            {x}

            *1E-12 ) could interfere with what we need to test.

            P.S. even generating a number as large as in 1E45 range give a valid result.
            992964467961790114603370108662767761814978560.0E3 = 9.9e47
            PHP math functions seem quite robust..

            Show
            ppichet Pierre Pichet added a comment - By the way, a formula like {x}E-12 COULD work when it comes to attempting the question. The {x} will be converted to the numerical to which the exponent will be applied as E number style is allowed in PHP 2.4E3 = 2400 I use this here to test the conversion process as any operation (ie. {x} *1E-12 ) could interfere with what we need to test. P.S. even generating a number as large as in 1E45 range give a valid result. 992964467961790114603370108662767761814978560.0E3 = 9.9e47 PHP math functions seem quite robust..
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated (blind mode). Thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (blind mode). Thanks!
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Tested. Passed. Thanks

            Show
            samhemelryk Sam Hemelryk added a comment - Tested. Passed. Thanks
            Hide
            nebgor Aparup Banerjee added a comment -

            fixes have been rolled merrily up the stream! Thanks everybody!

            Show
            nebgor Aparup Banerjee added a comment - fixes have been rolled merrily up the stream! Thanks everybody!

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  10/Oct/11