Small floats changed to 0 when stored in TEXT cols in the DB - this breaks numerical questions

Weird. 3e-4 works. 3e-5 becomes 0.0. Definitely a bug.

Pierre. Any chance you could take a look at this? Thanks.

I take a look

The answerdata is correctly decode for example 1e-06 but store as 0.00 in the database answer field which is defined as TEXT
So this is a conversion problem as when the value return from the $this->apply_unit
$answer->answer = $this->apply_unit($answerdata, $units);
it is return as a float

                if (isset($tmpunits[$responseparts[5]])) {
                    // Valid number with unit.
                    return (float)$responseparts[1] / $tmpunits[$responseparts[5]];
                } else {
                    // Valid number with invalid unit. Must be wrong.
                    return false;
                }

            } else {
                // Valid number without unit.
                return (float)$responseparts[1];
            }

If the value is less than 1e-4 (i.e 1e-6) it is stored as 0.0.
However there is no problem with the tolerance, the tolerance field is defined as VARCHAR(255)

this is an old problem that does not occur often as few people use so low values and this code in apply_unit() is quite old...

So Tim I give it back to you as you are the database expert

In my case values are that low because they are capacities in Farad. Surely a non common situation, but also not so rare.

Thanks for the analysis Pierre. That should make it easier to fix.

I created some unit tests that show that this problem is in the database layer:
https://github.com/timhunt/moodle/compare/master...MDL-27325_wip
The last test, storing 1e-5 in a TEXT column, fails for me. The other pass.

Adding Eloy, as the DB expert. What do you make of this Eloy?

I think we should come back to the initial saving code

            if (trim($answerdata) === '*') {
                $answer->answer = '*';
            } else {
                $answer->answer = $this->apply_unit($answerdata, $units);
                if ($answer->answer === false) {
                    $result->notice = get_string('invalidnumericanswer', 'quiz');
                }
            }

The use of applyunit is to test if the number is a real number.
However the applyunit() code first use is to check the student response which could have unit use as it for the teacher answer

    /**
     * Checks if the $rawresponse has a unit and applys it if appropriate.
     *
     * @param string $rawresponse  The response string to be converted to a float.
     * @param array $units         An array with the defined units, where the
     *                             unit is the key and the multiplier the value.
     * @return float               The rawresponse with the unit taken into
     *                             account as a float.
     */
    function apply_unit($rawresponse, $units) {

        // Make units more useful
        $tmpunits = array();
        foreach ($units as $unit) {
            $tmpunits[$unit->unit] = $unit->multiplier;
        }
        // remove spaces and normalise decimal places.
        $rawresponse = trim($rawresponse) ;
        $search  = array(' ', ',');
        // test if a . is present or there are multiple , (i.e. 2,456,789 ) so that we don't need spaces and ,
        if ( strpos($rawresponse,'.' ) !== false || substr_count($rawresponse,',') > 1 ) {
            $replace = array('', '');
        }else { // remove spaces and normalise , to a . .
            $replace = array('', '.');
        }
        $rawresponse = str_replace($search, $replace, $rawresponse);


        // Apply any unit that is present.
        if (ereg('^([+-]?([0-9]+(\\.[0-9]*)?|\\.[0-9]+)([eE][-+]?[0-9]+)?)([^0-9].*)?$',
                $rawresponse, $responseparts)) {
                echo"<p> responseparts <pre>";print_r($responseparts) ;echo"</pre></p>";

            if (!empty($responseparts[5])) {

                if (isset($tmpunits[$responseparts[5]])) {
                    // Valid number with unit.
                    return (float)$responseparts[1] / $tmpunits[$responseparts[5]];
                } else {
                    // Valid number with invalid unit. Must be wrong.
                    return false;
                }

            } else {
                // Valid number without unit.
                return (float)$responseparts[1];
            }
        }
        // Invalid number. Must be wrong.
        return false;
    }

we should
redesign a more specific test
or
if apply_unit result is not false convert the apply_unit return to a standard number format.

I will test this last solution and come back later.

the solution is the following

@@ -180,15 +180,17 @@ class question_numerical_qtype extends question_shortanswer_qtype {
                 $answer->id = $DB->insert_record('question_answers', $answer);
             }
 
             if (trim($answerdata) === '*') {
                 $answer->answer = '*';
-            } else {
+            } else {                
                 $answer->answer = $this->apply_unit($answerdata, $units);
                 if ($answer->answer === false) {
                     $result->notice = get_string('invalidnumericanswer', 'quiz');
-                }
+                }else {
+                   $answer->answer = sprintf("%E",$answer->answer);
+                } 
             }
             $answer->fraction = $question->fraction[$key];
             $answer->feedback = $this->import_or_save_files($question->feedback[$key],
                     $context, 'question', 'answerfeedback', $answer->id);

notice that this transforms the value following the sprintf options which should not include the $location parameter as we want it to be seen as a float when used in the
function test_response(&$question, &$state, $answer ) {
$answer->min <= $response && $response <= $answer->max

However the user will not necesseraly recognize what he has typed.

The grading is Ok tested with
100% 0.0001
90% 0.00001
80% 0.000001
and so on.
The conversion is not necessary for the tolerance.

So this work but details remain to be defined and
Tim you should notice that this is the same code in new engine

Getting this for cross-db testing and analysis, thanks everybody!

Wow, the problem is not Moodle specific but MySQL, I just tried this from console:

Wow, the problem is not Moodle specific but MySQL, I just tried this from console:

mysql> INSERT INTO  mdl_config (name, value) VALUES ('1e-1',  1e-1);
Query OK, 1 row affected (0.00 sec)

mysql> INSERT INTO  mdl_config (name, value) VALUES ('1e-2',  1e-2);
Query OK, 1 row affected (0.00 sec)

mysql> INSERT INTO  mdl_config (name, value) VALUES ('1e-3',  1e-3);
Query OK, 1 row affected (0.00 sec)

mysql> INSERT INTO  mdl_config (name, value) VALUES ('1e-4',  1e-4);
Query OK, 1 row affected (0.00 sec)

mysql> INSERT INTO  mdl_config (name, value) VALUES ('1e-5',  1e-5);
Query OK, 1 row affected (0.00 sec)

mysql> select * from mdl_config where name like '1e-%';
+-----+------+-------+
| id  | name | value |
+-----+------+-------+
| 430 | 1e-1 | 0.10  |
| 431 | 1e-2 | 0.01  |
| 432 | 1e-3 | 0.00  |
| 433 | 1e-4 | 0.00  |
| 434 | 1e-5 | 0.00  |
+-----+------+-------+
5 rows in set (0.00 sec)

I think we can, both on insert and update, convert those floats to strings when the target column is a TEXT column, does that sound ok? Horrible MySQL rounding cast!

Hehe, and it not only happens with TEXT columns but also with VARCHAR ones, but for some strange circumstance, in a different way. The objective is to keep the original 'xe-y' stored in database, without any conversion, correct?

from MySQL
requirements. See Section 10.5, “Data Type Storage Requirements”.

BLOB values are treated as binary strings (byte strings). They have no character set, and sorting and comparison are based on the numeric values of the bytes in column values. TEXT values are treated as nonbinary strings (character strings). They have a character set, and values are sorted and compared based on the collation of the character set.

You may find MySQL's string handling functions useful for working with such data. See Section 11.5, http://dev.mysql.com/doc/refman/5.0/en/string-functions.html#function_format
Formats the number X to a format like '#,###,###.##', rounded to D decimal places, and returns the result as a string. If D is 0, the result has no decimal point or fractional part. D should be a constant value.

mysql> SELECT FORMAT(12332.123456, 4);
-> '12,332.1235'
mysql> SELECT FORMAT(12332.1,4);
-> '12,332.1000'
mysql> SELECT FORMAT(12332.2,0);
-> '12,332'
If there is a character set, there should not be somewhere a way to set the number format other than using the MySQL specific FORMAT function (which we cannot do on Moodle ), when creating the field in MySQL?

on the collation of the character set
This could mean that if the collation is set to french the numbers will be set as 123,34 if they are not converted before storing.
The numbers should be converted before storing or perhaps we should use BLOB ?

Pierre, I think we should store the tolerance in the database as a float, so if we are going to accept locale-numbers from the input form, we have to convert them to float before we save the question.

Eloy, I think we do have to work-around this MySQL bug as you suggest. Thanks.

Eloy's changes reviewed. +1 from me.

Tim,
I agree with the preconversion to text but I worried about the
Horrible MySQL rounding cast! and try to see if there could be another solution.
I know well that it is not your coding policy

As tolerance is a real number, I agree that using float will eliminate possible problems but not all with the actual ( under revision )new engine proposal.
Take a look of MDL-27363.

Requesting for integration.

Tim, note I've ended rebasing the branches to get linear history on them, so your _wip above doesn't match anymore with final commits.

Ciao

Sorry Tim and Eloy for the not always pertinent comments as Pull is no more used and
I did not find readily that (and how ) Moodle bugs are now handled on GitHub.

Pierre

Thanks all involved, this has been integrated now.

Cheers
Sam

I didn't want to fail this because I was concerned I misunderstood the Testing instructions. Maybe someone else can confirm / reject my results.

Followed instructions but when i got to 'Re-edit it' step I found that 1E-6 had turned into 1.E-6 and 5E-7 had turned into 5.E-7

Also, got the following errors on unit testing:
Fail: lib/dml/simpletest/testdml.php / ▶ dml_test / ▶ test_unique_index_collation_trouble
Unique index is accent insensitive, this may cause problems for non-ascii languages. This is usually caused by accent insensitive default collation. at [/Applications/MAMP/htdocs/moodle20integrations/lib/dml/simpletest/testdml.php line 2978]

Fail: lib/dml/simpletest/testdml.php / ▶ dml_test / ▶ test_sql_binary_equal
SQL operator "=" is expected to be accent sensitive at [/Applications/MAMP/htdocs/moodle20integrations/lib/dml/simpletest/testdml.php line 3006]

Fail: lib/dml/simpletest/testdml.php / ▶ dml_test / ▶ test_sql_binary_equal
SQL operator "=" is expected to be case sensitive at [/Applications/MAMP/htdocs/moodle20integrations/lib/dml/simpletest/testdml.php line 3009]

2/2 test cases complete: 1068 passes, 3 fails and 1 exceptions.
Run at Tuesday, 10 May 2011, 03:08 pm. Time taken: 26 secs. Using SimpleTest version 1.0.1.

Those unit test failures are known bugs in MySQL, that we have not found a way to work-around.

1.E-6 and 1E-6 are exactly the same value. The fact that the number format gets normalised it add the optional . is entirely fine. It is just like the fact that if you type " frog " as the answer to a short-answer question, Moodle will tidy that up to "frog" without the leading or trailing whitespace. So, if you have not found anything else, this is a pass.

Yeah, 100% agree with Tim, that is what I was trying to write in the testing instructions:

• TEST: Re-edit it: the answer should be shown as originally set (1E-6) or some numerically equivalent expression. (i.e. 1.E-6 and 1E-6)
• TEST: Run the DB unit tests (Admin->Development->Functional DB tests) against all the DB drivers (mysqli, pgsql, mssql, sqlsrv and oci). No failures/exception should happen in the test_insert_record, test_update_record and test_set_field unit tests. (i.e. errors above happen in other tests)

Sorry for not stating it clearly. So yes, it's a pass for me too. Ciao

PS: And big thanks!

Passing as requested by Eloy "I think it's 100% safe to do so, as commented by Tim and me there"

Thanks glenntimhelen!

Everybody say: "I love MySQL, I love MySQL".

Many thanks, closing!