-
Notifications
You must be signed in to change notification settings - Fork 68
perf: use xpress c interface, xpress solution order of operations improvement, remove writing of redundant solution files #497
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
Conversation
123eb87 to
46f846e
Compare
FabianHofmann
left a comment
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.
that was fast ;)
I very much like that, and I honestly have the feeling that this is a redundant artefact from an older implementation. So I would also be happy to remove the writing of the sol file entirely if not needed by us. |
51f474e to
42369b3
Compare
|
I've pushed a small change that should remove the writing of the solution file in situations where it isn't needed or requested. For now, I've left the concept in there in case people are relying on the |
linopy/model.py
Outdated
| if ( | ||
| solver_name | ||
| in ["xpress", "gurobi", "highs", "mosek", "scip", "copt", "mindopt"] | ||
| and not keep_files | ||
| ): | ||
| # these (solver, keep_files=False) combos do not need a solution file | ||
| solution_fn = None | ||
| else: | ||
| solution_fn = self.get_solution_file() |
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.
I would like to avoid this hard-coded list of solvers here, rather define is in the "preamble" of solvers.py and import from there or add the logic to the solver classes (would however mean to pass keep_files to them)
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.
Thanks! Have done the first option for a smaller set of changes. I originally thought about passing the keep_files parameter through to the solvers but it felt a little messy.
FabianHofmann
left a comment
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.
great @CharlieFModo
Closes # (if applicable).
Changes proposed in this Pull Request
Xpress._solve_problem_from_fileproblem.getConstraints()etc. is expensive and so is accessing the names via.name.getDualsbefore doing the expensive constraint name extractionwritebinsolfor a c. 10x improvement fromwritesolkeep_files=FalseChecklist
doc.doc/release_notes.rstof the upcoming release is included.