Closed GoogleCodeExporter closed 9 years ago
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
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
> 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
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
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:
(Updated patch with fixed indentation)
Original comment by phillip....@googlemail.com
on 2 Apr 2013 at 7:12
Attachments:
This issue was closed by revision r2213.
Original comment by yarmond
on 3 Apr 2013 at 11:10
Original issue reported on code.google.com by
phillip....@googlemail.com
on 28 Mar 2013 at 12:02