-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Booster read/write locale dependency #2891
Fix Booster read/write locale dependency #2891
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@StrikerRUS any style issues? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AlbertoEAF Please fix the following cpplint errors:
Changed everything you requested @StrikerRUS :) Anything else? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AlbertoEAF Thank you very much for fixing all linting issues!
Thanks for merging @StrikerRUS :) |
This reverts commit 6b68967 (microsoft#2891) and fixes microsoft#2979.
This request fixes the bug mentioned on the issue #2890.
In C++, the process is by default is initialized to use the minimal "C" locale, however, in Java, when using the SWIG wrappers, this default is broken and the current machine locale is used instead.
Since during model read/write we rely on many C/C++ functions, some of which are 100% dependant on the locale, such as
std::stod()
, overriding the locale to a sane default is required.Instead of setting the locale globally across the full process, as we might want to still read/write datasets according to the locale, a class to manage the locale context
LocaleContext
was created.We only activate/deactivate that context (to force the "C" locale) during model reads/writes, to ensure our model format is independent of any locale changes in the machine or calling processes.
Motivation for this:
To avoid such issues, the fix should be done on the core library, so no bindings can be affected by locale issues, whatever happens, and this way the model format is always consistent.