Skip to content

Add timelimit error handling to pyscipopt adapter #427

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

Merged
merged 9 commits into from
Apr 23, 2025

Conversation

taqro
Copy link
Contributor

@taqro taqro commented Apr 22, 2025

Added error handling for timelimit to ommx_pyscipopt_adapter.

@taqro taqro changed the title add timelimit error handling Add timelimit error handling to pyscipopt adapter Apr 22, 2025
takuro saito added 2 commits April 23, 2025 08:19
@taqro taqro requested a review from Copilot April 22, 2025 23:29
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds timelimit error handling to the pyscipopt adapter and includes an integration test to verify the behavior.

  • Introduces error handling for timelimit status in both the decode() and decode_to_state() methods in the adapter
  • Adds a new integration test in test_integration.py to check that InfeasibleDetected is raised when no solution is found within a tight time limit

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
python/ommx-pyscipopt-adapter/tests/test_integration.py Added integration test to ensure timelimit handling triggers an exception when no solutions are found
python/ommx-pyscipopt-adapter/ommx_pyscipopt_adapter/adapter.py Added timelimit error handling branches that throw an exception or issue a warning based on the number of solutions found
Comments suppressed due to low confidence (1)

python/ommx-pyscipopt-adapter/ommx_pyscipopt_adapter/adapter.py:209

  • [nitpick] Consider adding tests to cover the timelimit scenario in the decode() method to ensure its behavior is consistent with decode_to_state().
if data.getStatus() == "timelimit":

@taqro taqro requested a review from Copilot April 22, 2025 23:34
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds timelimit error handling to the pyscipopt adapter with accompanying integration tests.

  • Added two integration tests to verify error handling when the solver stops due to a timelimit.
  • Updated the decode and decode_to_state methods in the adapter to raise an error when no solutions are found and to issue a warning when solutions are available despite a time limit.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
python/ommx-pyscipopt-adapter/tests/test_integration.py Adds integration tests for timelimit error handling.
python/ommx-pyscipopt-adapter/ommx_pyscipopt_adapter/adapter.py Implements timelimit error handling by raising errors or warnings depending on available solutions.

@taqro taqro marked this pull request as ready for review April 22, 2025 23:45
@taqro taqro requested a review from NY57 April 23, 2025 00:16
taqro and others added 2 commits April 23, 2025 11:39
Co-authored-by: NY57 <121103446+NY57@users.noreply.github.com>
Copy link
Member

@NY57 NY57 left a comment

Choose a reason for hiding this comment

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

LGTM

@NY57 NY57 merged commit e25fd79 into main Apr 23, 2025
35 checks passed
@NY57 NY57 deleted the feature/timelimit_error_handling branch April 23, 2025 05:18
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