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

Xpress fixes #3576

Merged
merged 23 commits into from Apr 25, 2023
Merged

Xpress fixes #3576

merged 23 commits into from Apr 25, 2023

Conversation

djunglas
Copy link
Contributor

This patches fixes a lot of different issues in the Xpress connector of Google OR tools.

@google-cla
Copy link

google-cla bot commented Nov 30, 2022

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.

@lperron
Copy link
Collaborator

lperron commented Nov 30, 2022

Thanks for the pr. You need to sign the CLA.

@lperron
Copy link
Collaborator

lperron commented Dec 5, 2022

Ping.

I cannot accept the PR if you do not sign the CLA.

Thanks

@djunglas
Copy link
Contributor Author

djunglas commented Dec 5, 2022

Ping.

I cannot accept the PR if you do not sign the CLA.

Thanks

Ack. I am working on this but it is not that easy. I first have to clarify some things with my employer.

@lperron
Copy link
Collaborator

lperron commented Dec 5, 2022 via email

ortools/linear_solver/xpress_interface.cc Outdated Show resolved Hide resolved
ortools/linear_solver/xpress_interface.cc Outdated Show resolved Hide resolved
ortools/linear_solver/xpress_interface.cc Outdated Show resolved Hide resolved
ortools/linear_solver/xpress_interface.cc Show resolved Hide resolved
ortools/linear_solver/xpress_interface.cc Outdated Show resolved Hide resolved
ortools/linear_solver/xpress_interface.cc Outdated Show resolved Hide resolved
We also support reading parameters from a file, even though Xpress does
not support parameter files itself.
The format for parameters is always NAME=VALUE. If set from a string,
settings must be separated by ';'. If read from a file, there can be at
most one setting per line.
At the moment all logging goes to the standard output. That should be
enough for debugging.
We must always query the dimensions of the original model.
This can never be correct.
This is more future proof than removing all message callbacks.
That control on its own has no effect anyway.
On the other hand, we now print through the message callback unless we
are requested to be `quiet()`.

`XPRS_OUTPUTLOG` could be set via
`SetSolverSpecificParametersAsString()` if deemed necessary by the user.
This was requested by the reviewer.
Note that we pass around the `XPRSprob` instance rather than passing the
`XpressInterface` instance and then calling functions like
`underlying_solver()` on it. That seemed more reasonable, especially in
light of the existing `XPRSgetnumcols()` and friends.
This makes the `virtual` keyword obsolete and also the code more
readable.
Instead of logging to `std::cout`, log to the appropriate channels.
@djunglas
Copy link
Contributor Author

Hello @lperron,
supposedly my company just managed to sign a company wide CLA that should cover my contributions here. Sorry for the very long time it took to do that.
I am not sure whether I have to do more. I fixed all the things that @flomnes found. I can see that some automated tests are failing, but as far as I can see, these failures are unrelated to my changes. Should I do something about them? Should I maybe rebase my branch on main rather than stable?

@lperron lperron merged commit d36cad9 into google:main Apr 25, 2023
117 of 128 checks passed
@lperron
Copy link
Collaborator

lperron commented Apr 25, 2023

Great news. This is merged in main.

@lperron
Copy link
Collaborator

lperron commented Apr 25, 2023 via email

@pet-mit
Copy link
Contributor

pet-mit commented May 4, 2023

Hello @djunglas, have you seen our work on the XPRESS interface here?
We think it would be beneficial for us all to work together on this subject. Would you like to discuss this?

@flomnes @sgatto @klorel

@djunglas
Copy link
Contributor Author

djunglas commented May 5, 2023

Hello @pet-mit, no, I did not see your work. Yes, I would like to discuss things. Let's get in contact.

@pet-mit
Copy link
Contributor

pet-mit commented May 9, 2023

Hello @djunglas
Great to hear it! I sent you a message on linkedin

@Mizux Mizux self-assigned this May 14, 2023
@Mizux Mizux added this to the v9.7 milestone May 14, 2023
@Mizux Mizux added Feature Request Missing Feature/Wrapper Solver: XPRESS XPRESS Solver related issue labels May 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Missing Feature/Wrapper Solver: XPRESS XPRESS Solver related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants