Numbers in .cti files get parsed depending on locale setting

[GoogleCodeExporter]

I recently noticed this:

--------------------------------------
$ ipython
In [1]: from Cantera import GRI30

In [2]: GRI30()

**** WARNING ****
For species H2, discontinuity in cp/R detected at Tmid = 1000
    Value computed using low-temperature polynomial:  -6,998e+12.
    Value computed using high-temperature polynomial: 1,999e+12.

...
--------------------------------------

and traced the problem to ipython's QT backend calling setlocale(), which among 
others localizes calls to atof(), in my case to de_DE, where the decimal 
delimiter is not "." but "," and therefore all numbers were effectively 
floor()ed.

I already filed a bug against IPython (In my opinion, this is primarily a 
problem of their QT backend), but to avoid further problems for others I 
suggest that you either override the locale (see setlocale(3)) or issue a 
warning on startup when a non-standard locale is set.

Original issue reported on code.google.com by phillip....@googlemail.com on 28 Mar 2013 at 12:02

[GoogleCodeExporter]

For completeness, here are the links to the other bug reports:
 https://github.com/matplotlib/matplotlib/issues/1867
 https://github.com/ipython/ipython/issues/3103

Original comment by phillip....@googlemail.com on 28 Mar 2013 at 1:40

[GoogleCodeExporter]

Fascinating. Can you confirm that you get the same behavior with running:

    import Cantera
    Cantera.IdealGasMix('gri30.xml')

I think the following change might fix this problem: In src/base/xml.cpp, add 
the following in the constructor for XML_Reader, near line 130:

#include <clocale>
std::setlocale(LC_NUMERIC, "C");

Original comment by yarmond on 28 Mar 2013 at 8:39

• Changed state: Accepted

[GoogleCodeExporter]

> Can you confirm that you get the same behavior with running

Yes, I do.

> I think the following change might fix this problem: In src/base/xml.cpp, add 
the 
> following in the constructor for XML_Reader, near line 130:

I also think so, but other parts of the code where atof() is used will still be 
broken, at least in the sense that there will be a race with other libraries in 
the same program resetting the locale setting to non-C in between the 
XML_Reader constructor call and later calls to atof().)

Github user mdboom commented [1] that one should better not use locale aware 
APIs at all for parsing, so maybe replacing atof() with another function would 
be the better solution?! If you want to stick to atof() you could also use it 
like this [2]:

 double atof_C(const char * input) {
  const char * oldlocale = setlocale(LC_NUMERIC, "C");
  double result = atof(input);
  setlocale(LC_NUMERIC, oldlocale);
  return result;
 }

[1] https://github.com/matplotlib/matplotlib/issues/1867#issuecomment-15589615
[2] 
http://gruppen.niuz.biz/locale-t261800.html?s=42b244d71071188fdcb3ef9396bd4a24

Original comment by phillip....@googlemail.com on 29 Mar 2013 at 12:06

[GoogleCodeExporter]

Yes, you're right, my suggested change was mostly a 'band-aid' that I wanted to 
see if worked, as I was having trouble reproducing this error on my computer. I 
have since been able to replicate this behavior (I needed to install a 
non-English locale first).

Your proposed solution still has a race condition where other threads might 
change the locale between the call to setlocale and the call to atof. I think 
it would be better to use a std::stringstream, which can be "imbued" with the 
correct locale [1]. The atofCheck function could be updated to use a 
stringstream to do the conversion (like this: [2]), and the remaining calls to 
atof can be replaced with calls to atofCheck.

[1] http://www.cplusplus.com/reference/ios/ios_base/imbue/
[2] http://www.cplusplus.com/forum/articles/9645/

Original comment by yarmond on 29 Mar 2013 at 3:33

• Changed state: PlanMade

[GoogleCodeExporter]

I assumed libstdc++'s stringstream would use the C API internally and thous 
suffer from the same problem, but I've apparently been wrong (See 
bits/locale_facets.tcc, _M_extract_float(..)).

I've tried replacing all atof() calls with calls to atofCheck(), but this at 
least breaks parsing mass fraction strings. For example, when "CH4:1,O2:2" is 
parsed, atofCheck("1,") fails. So I instead replaced all atof() calls with ones 
to fpValue() and updated fpValue() to use sstream for parsing. (I'll attach the 
patch.)

Now the bug is gone and everything still seems to work. All tests in "scons 
test" still pass.

Original comment by phillip....@googlemail.com on 2 Apr 2013 at 7:09

Attachments:

• replace-atof-with-sstream.diff

[GoogleCodeExporter]

(Updated patch with fixed indentation)

Original comment by phillip....@googlemail.com on 2 Apr 2013 at 7:12

Attachments:

• replace-atof-with-sstream.diff

[GoogleCodeExporter]

This issue was closed by revision r2213.

Original comment by yarmond on 3 Apr 2013 at 11:10

• Changed state: Fixed