Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds a Pyomo-based adapter for OMMX v1 instances, with full solve, decode, and error‐handling support. It also includes comprehensive integration and error tests, plus project metadata and task automation.
- Implement
OMMXPyomoAdapterwith variable, objective, and constraint mapping, plus solve/decode logic - Add integration tests covering LP, MILP, binary, maximize, constant, quadratic objectives/constraints
- Add error tests for unsupported objectives/constraints, solver availability, and infeasible constants
- Configure project metadata (
pyproject.toml) and automation tasks (Taskfile.yml)
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_integration.py | New integration tests for various problem types |
| tests/test_error.py | New tests asserting adapter errors for unsupported features and infeasible cases |
| tests/test_adapter.py | Basic adapter solve test verifying solution optimality |
| pyproject.toml | Project metadata, version bump, dependencies, and classifiers |
| ommx_pyomo_adapter/exception.py | Define OMMXPyomoAdapterError |
| ommx_pyomo_adapter/adapter.py | Implement main adapter logic (solve, decode, variable/objective/constraint setup) |
| ommx_pyomo_adapter/init.py | Expose public API |
| Taskfile.yml | Define tasks for testing, linting, type checking, and default workflow |
Comments suppressed due to low confidence (2)
python/ommx-pyomo-adapter/tests/test_integration.py:11
- [nitpick] The parameter name 'generater' is misspelled; consider renaming it to 'generator' for clarity.
"generater",
python/ommx-pyomo-adapter/pyproject.toml:19
- The classifiers list both Apache Software License and MIT License, which may conflict; choose a single license to accurately reflect the project's licensing.
"License :: OSI Approved :: Apache Software License",
| solver.solve(model) | ||
| # check objective | ||
| assert pyo.value(model.objective) == 0 | ||
| state = adapter.decode_to_state(model) |
There was a problem hiding this comment.
decode_to_state expects SolverResults, not a Pyomo model; store the solver result in a variable and pass it to decode_to_state (e.g., results = solver.solve(model); state = adapter.decode_to_state(results)).
| solver.solve(model) | |
| # check objective | |
| assert pyo.value(model.objective) == 0 | |
| state = adapter.decode_to_state(model) | |
| results = solver.solve(model) | |
| # check objective | |
| assert pyo.value(model.objective) == 0 | |
| state = adapter.decode_to_state(results) |
| raise OMMXPyomoAdapterError( | ||
| f"Variable {var_name} has no value - model may not be solved" | ||
| ) | ||
| entries[var.id] = value | ||
| return State(entries=entries) | ||
| except Exception as e: | ||
| raise OMMXPyomoAdapterError(f"Failed to decode state from results: {e}") |
There was a problem hiding this comment.
This specific error message is raised for missing variable values, but tests expect a generic 'Failed to decode state from results' message. Consider allowing the exception to be caught by the surrounding except block or updating the message format to match test expectations.
| raise OMMXPyomoAdapterError( | |
| f"Variable {var_name} has no value - model may not be solved" | |
| ) | |
| entries[var.id] = value | |
| return State(entries=entries) | |
| except Exception as e: | |
| raise OMMXPyomoAdapterError(f"Failed to decode state from results: {e}") | |
| raise ValueError( | |
| f"Variable {var_name} has no value - model may not be solved" | |
| ) | |
| entries[var.id] = value | |
| return State(entries=entries) | |
| except Exception as e: | |
| raise OMMXPyomoAdapterError("Failed to decode state from results") from e |
No description provided.