Moodle
  1. Moodle
  2. MDL-28202

Edge case missing from Calculated qtype

    Details

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

      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.

        Activity

        Hide
        Tim Hunt added a comment -

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

        Show
        Tim Hunt added a comment - Would be nice to see some additional unit tests to cover this case.
        Hide
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        Eloy Lafuente (stronk7) added a comment -

        Integrated (blind mode). Thanks!

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

        Tested. Passed. Thanks

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

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

        Show
        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: