Skip to content
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

Use absl's locale-independent conversion functions in XPRESS MPSolver interface #4382

Merged
merged 2 commits into from
Sep 21, 2024

Conversation

flomnes
Copy link
Contributor

@flomnes flomnes commented Sep 20, 2024

Using such functions as std::stoi, std::stod, etc. caused us a lot of trouble because these functions are locale dependent, as illustrated below

#include <iostream>
#include <locale>
#include <string>

void convert(std::string locale)
{
    std::string value = "9.81";
    if (std::setlocale(LC_ALL, locale.c_str()))
    {
        double converted = std::stod(value);
        std::cout << "locale " << locale << " converted to " << converted << std::endl;
    }
    else
    {
        std::cout << "locale set failed for " << locale << std::endl;
    }
}
int main()
{
    convert("C");
    convert("fr_FR.utf8");
    // OUTPUT
    // locale C converted to 9.81
    // locale fr_FR.utf8 converted to 9
}

At first, we thought that using ScopedLocale would allow us to temporarily use the "C" locale, but this approach has multiple problems

  1. It's not thread-safe. For example calling std::stod when the locale is being changed through std::setlocale is undefined behavior source.
  2. C is hard, const char* pointers easily become dangling or invalid without the caller knowing
struct ScopedLocale {
  ScopedLocale() {
    oldLocale = std::setlocale(LC_NUMERIC, nullptr);
    auto newLocale = std::setlocale(LC_NUMERIC, "C");
    CHECK_EQ(std::string(newLocale), "C");
  }
  ~ScopedLocale() { std::setlocale(LC_NUMERIC, oldLocale); }

 private:
  const char* oldLocale;
};

We ran into an issue where the locale wasn't properly restored after using MPSolver::XpressInterface. So we decided to use absl's locale independent functions to perform that task.

OMNES Florian and others added 2 commits September 20, 2024 16:22
Use absl's locale-independent conversion functions in XPRESS interface
Copy link

google-cla bot commented Sep 20, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@flomnes flomnes changed the title Use absl's locale-independent conversion functions in XPRESS interface Use absl's locale-independent conversion functions in XPRESS MPSolver interface Sep 20, 2024
@Mizux Mizux added Bug Solver: Linear Solver Related to all Linear Solver (GLOP, BOP, CBC etc...) labels Sep 20, 2024
@Mizux Mizux added this to the v10.0 milestone Sep 20, 2024
@lperron lperron merged commit 9f41024 into google:main Sep 21, 2024
65 of 150 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Solver: Linear Solver Related to all Linear Solver (GLOP, BOP, CBC etc...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants