Skip to content

Conversation

@chris-maes
Copy link
Contributor

This PR adds the string parameter CUOPT_USER_PROBLEM_FILE = "user_problem_file". By default the parameter is the empty string "". If the parameter is set by the user to something other than the empty string, we will write out an MPS file containing the LP or MIP.

This is very useful when trying to debug interfaces like CVXPY where the interface between CVXPY and cuOpt may have performed transformation to the user problem. This allows us to reproduce failures on the engine side directly from the MPS file without needing Python scripts.

@chris-maes chris-maes added this to the 25.05 milestone Jun 2, 2025
@chris-maes chris-maes self-assigned this Jun 2, 2025
@chris-maes chris-maes requested a review from a team as a code owner June 2, 2025 19:49
@chris-maes chris-maes added non-breaking Introduces a non-breaking change improvement Improves an existing functionality labels Jun 2, 2025
@copy-pr-bot
Copy link

copy-pr-bot bot commented Jun 2, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@rgsl888prabhu
Copy link
Collaborator

@chris-maes chris-maes requested a review from a team as a code owner June 2, 2025 20:45
@chris-maes chris-maes requested a review from tmckayus June 2, 2025 20:45
@chris-maes
Copy link
Contributor Author

@chris-maes Lets add this to https://github.com/NVIDIA/cuopt/blob/branch-25.05/docs/cuopt/source/cuopt-c/lp-milp/lp-milp-c-api.rst#parameter-constants and https://github.com/NVIDIA/cuopt/blob/branch-25.05/docs/cuopt/source/lp-milp-settings.rst

And is this limited to C API or are you adding changes to python as well ?

I added docs to the files you suggested above. The parameter should be supported across all the APIs. I made changes to add it as named constant to Python. Hopefully, it should be picked up automatically on the server side (since I send all the parameters over the wire in ThinClientSolverSettings. But maybe other changes need to be made on the server side. (I'm less familiar with how that works.)

@chris-maes
Copy link
Contributor Author

/ok to test 89133fc

Copy link
Collaborator

@rgsl888prabhu rgsl888prabhu left a comment

Choose a reason for hiding this comment

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

Lets add a test for this parameter

@chris-maes
Copy link
Contributor Author

@chris-maes You would want to add changes here https://github.com/rapidsai/cuopt/blob/6499e52f8d48795b3373c14825d0f9b00cf84313/python/cuopt_self_hosted/cuopt_sh_client/thin_client_solver_settings.py#L167

No change is required in thin_client_solver_settings.py as all entries in parameter_dict are added to solver_config

https://github.com/rapidsai/cuopt/blob/6499e52f8d48795b3373c14825d0f9b00cf84313/python/cuopt_server/cuopt_server/utils/linear_programming/data_definition.py#L386

https://github.com/rapidsai/cuopt/blob/6499e52f8d48795b3373c14825d0f9b00cf84313/python/cuopt_server/cuopt_server/utils/linear_programming/solver.py#L211

Please let me know if it's confusing, I will update your PR.

In general, I don't think we should need to make changes in data_defintion.py and solver.py to add in a new parameter. This makes it difficult to add new parameters as you are required to add code in many different files. We should minimize the number of files you need to touch to add a parameter. I think the C++ code should be the source of truth of what parameters are accepted and the server should just act as a thin layer on top of C++. But that is perhaps a discussion for another day.

Thinking more about Server. I don't know if it is appropriate to add the parameter to the Server API. This parameter will cause the solver to write out a file. If the solver is on a different machine than the client, there would need to be some mechanism to send the file back to the client.

Maybe for now, we should just add this to the CLI, C, and Python APIs. And not support the parameter on the Server.

Copy link
Contributor

@aliceb-nv aliceb-nv left a comment

Choose a reason for hiding this comment

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

Thank you Chris!

@rgsl888prabhu
Copy link
Collaborator

@chris-maes You would want to add changes here https://github.com/rapidsai/cuopt/blob/6499e52f8d48795b3373c14825d0f9b00cf84313/python/cuopt_self_hosted/cuopt_sh_client/thin_client_solver_settings.py#L167

No change is required in thin_client_solver_settings.py as all entries in parameter_dict are added to solver_config

https://github.com/rapidsai/cuopt/blob/6499e52f8d48795b3373c14825d0f9b00cf84313/python/cuopt_server/cuopt_server/utils/linear_programming/data_definition.py#L386

https://github.com/rapidsai/cuopt/blob/6499e52f8d48795b3373c14825d0f9b00cf84313/python/cuopt_server/cuopt_server/utils/linear_programming/solver.py#L211
Please let me know if it's confusing, I will update your PR.

In general, I don't think we should need to make changes in data_defintion.py and solver.py to add in a new parameter. This makes it difficult to add new parameters as you are required to add code in many different files. We should minimize the number of files you need to touch to add a parameter. I think the C++ code should be the source of truth of what parameters are accepted and the server should just act as a thin layer on top of C++. But that is perhaps a discussion for another day.

Thinking more about Server. I don't know if it is appropriate to add the parameter to the Server API. This parameter will cause the solver to write out a file. If the solver is on a different machine than the client, there would need to be some mechanism to send the file back to the client.

Maybe for now, we should just add this to the CLI, C, and Python APIs. And not support the parameter on the Server.

My mistake, I missed that all other parameters copied directly under to_dict which are not tolerance.

Regarding writing to a file, it can be written to a shared mount and it is up to user to configure it. But may be we can push this support for server to 25.08 since it may not be essential. I will create a issue for us to support this on server, if we deem this is not much of a use in case of server, we can close that.

@chris-maes
Copy link
Contributor Author

/ok to test a94cbb7

@chris-maes
Copy link
Contributor Author

@aliceb-nv I made a few changes to your write_mps routine.

  1. If there is a variable with 0 objective and a lower bound of zero (and perhaps does not appear in any constraints) we need to write its bound out in the Bounds section otherwise we don't get the same problem in the MPS file as in memory.

  2. I added a unit test to write out the mps file for the doc example. But I couldn't get the same problem from the MPS file, until I wrote out that it was a maximization problem. And again wrote the lower bound of zero.

This is strange as variables should default to [0, inf) bounds in MPS files unless specified. But I think there are certain cases where we need to be explicit about the bounds.

Copy link
Collaborator

@rgsl888prabhu rgsl888prabhu left a comment

Choose a reason for hiding this comment

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

Thank you @chris-maes for the PR, have few questions and requests. Rest looks good.

@chris-maes
Copy link
Contributor Author

/ok to test 85d07d7

@rgsl888prabhu rgsl888prabhu merged commit 8939e34 into NVIDIA:branch-25.05 Jun 4, 2025
56 checks passed
@chris-maes chris-maes deleted the write_user_problem branch June 4, 2025 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants