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

Fix locale fix #843

Merged
merged 8 commits into from
May 5, 2024
Merged

Fix locale fix #843

merged 8 commits into from
May 5, 2024

Conversation

Joao-Dionisio
Copy link
Collaborator

@Joao-Dionisio Joao-Dionisio commented Apr 24, 2024

We were changing the locale for everything (locale.LC_ALL), when I think we only wanted locale.LC_NUMERIC. Changing it for everything created new problems:

from pyscipopt import Model

m = Model()
m.readProblem("inf_model.cip")
m.writeParams("params.set")

m2 = Model()
m.readProblem("inf_model.cip")
[scip_var.c:766] ERROR: constant in linear sum
[cons_linear.c:17252] ERROR: no luck in parsing linear sum '<r'[0,0,0]>[C] -0.0282485875706215<r[0,0,0]>[C] == 0'
[reader_cip.c:722] ERROR: syntax error when reading constraint (line: 216):
  [linear] <c1>: <r'[0,0,0]>[C] -0.0282485875706215<r[0,0,0]>[C] == 0;
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "src/pyscipopt/scip.pxi", line 5186, in pyscipopt.scip.Model.readProblem
  File "src/pyscipopt/scip.pxi", line 266, in pyscipopt.scip.PY_SCIP_CALL
OSError: SCIP: read error!

Problem in question:
inf_model.txt

This way it's still passing the locale check we set up, and this problem disappears.

It might be that this raises other issues, but I think it's better to deal with them as they appear since it's difficult to predict how different locales can cause problems.

Copy link
Member

@mmghannam mmghannam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks João! Maybe we should add a test with the instance that's faulty to make sure this doesn't get reintroduced later. What do you think?

@Joao-Dionisio
Copy link
Collaborator Author

Done @mmghannam! I also realized I forgot to add the locale stuff to write methods, so your suggestion ended up being very helpful :D

@mmghannam mmghannam enabled auto-merge (squash) May 5, 2024 08:29
@mmghannam mmghannam merged commit 001f423 into scipopt:master May 5, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants