Skip to content

feat: extend IIS calculation to support Xpress solver#473

Merged
FabianHofmann merged 4 commits intomasterfrom
xpress-iis
Jul 22, 2025
Merged

feat: extend IIS calculation to support Xpress solver#473
FabianHofmann merged 4 commits intomasterfrom
xpress-iis

Conversation

@FabianHofmann
Copy link
Collaborator

Summary

This PR extends the IIS (Irreducible Infeasible Set) calculation functionality in linopy to support the Xpress solver, in addition to the existing Gurobi support.

Changes

  • Extended compute_infeasibilities() method: Now auto-detects solver type and routes to appropriate IIS implementation
  • Added _compute_infeasibilities_xpress() method: Implements Xpress-specific IIS computation using xpress.iisall() API
  • Updated print_infeasibilities() method: Now works with both Gurobi and Xpress solvers
  • Comprehensive test coverage: Extended existing tests and added new test suite test_infeasibility.py
  • Updated documentation: All docstrings now reflect dual solver support
  • Maintained backward compatibility: Existing Gurobi-only code continues to work unchanged

Technical Implementation

  • Uses Xpress's iisall() method to compute all IIS in the model
  • Leverages getiisdata() API to extract constraint information from each IIS
  • Maps Xpress constraint objects back to linopy constraint indices
  • Handles multiple IIS scenarios (common with element-wise constraints)
  • Provides clear error messages for unsupported solvers

Test Plan

  • Simple infeasible model detection (both solvers)
  • Complex infeasible model detection (both solvers)
  • Multi-dimensional infeasible model detection (both solvers)
  • Error handling for unsupported solvers
  • Error handling for missing solver models
  • Feasible model IIS computation
  • Deprecated method compatibility
  • Integration with existing test suite (32 tests passed)

🤖 Generated with Claude Code

- Add support for Xpress solver IIS computation alongside existing Gurobi support
- Implement _compute_infeasibilities_xpress() method using xpress.iisall() API
- Update compute_infeasibilities() to auto-detect solver type and route appropriately
- Extend test coverage to include both Gurobi and Xpress in infeasibility tests
- Add comprehensive test suite in test_infeasibility.py with various scenarios
- Update documentation to reflect dual solver support in docstrings
- Maintain backward compatibility with existing Gurobi-only code

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@claude
Copy link
Contributor

claude bot commented Jul 10, 2025

Claude finished @FabianHofmann's task —— View job


PR Review Complete

Review Todo List:

  • Read and analyze the main changes in linopy/model.py
  • Review the new test file test/test_infeasibility.py
  • Check the minor change in test/test_optimization.py
  • Evaluate Python best practices and type safety
  • Assess xarray integration patterns
  • Review performance implications for large-scale problems
  • Check mathematical correctness of solver interfaces
  • Evaluate memory efficiency considerations
  • Assess test coverage and edge cases
  • Check consistency with existing codebase patterns
  • Provide comprehensive review feedback

Code Review

This PR successfully extends IIS (Irreducible Infeasible Set) calculation to support Xpress solver alongside Gurobi. The implementation is well-structured and maintains backward compatibility.

Strengths

1. Type Safety and Python Best Practices:

  • Good use of type hints (e.g., list[int] return type in compute_infeasibilities())
  • Proper error handling with descriptive messages
  • Clean separation of concerns between solver-specific implementations
  • Consistent docstring format following the established pattern

2. Mathematical Correctness:

  • Xpress implementation correctly uses iisall() and getiisdata() API
  • Proper handling of multiple IIS scenarios (common with element-wise constraints)
  • Correct mapping from Xpress constraint objects back to linopy constraint indices
  • Maintains consistency with Gurobi implementation approach

3. Performance Considerations:

  • Uses set() to efficiently collect unique constraint labels across multiple IIS
  • Avoids redundant computations by collecting all IIS in one pass
  • Minimal memory overhead for constraint object mapping

4. Test Coverage:

  • Comprehensive test suite covering multiple scenarios (simple, complex, multi-dimensional)
  • Tests for both supported solvers with parametrized tests
  • Edge case testing (feasible models, unsupported solvers, missing solver models)
  • Backward compatibility testing for deprecated methods

⚠️ Areas for Improvement

1. Type Safety Issues:

  • linopy/model.py:1345-1395: The _compute_infeasibilities_xpress method uses Any type annotations for several parameters in the getiisdata() call. Consider using more specific types if available from the Xpress API, or at least document expected types in comments.

2. Error Handling:

  • linopy/model.py:1389-1393: The try-except ValueError block silently skips constraints that can't be found. Consider logging this occurrence or adding a debug message to help with troubleshooting.

3. Memory Efficiency:

  • linopy/model.py:1358: Creating list(solver_model.getConstraint()) loads all constraints into memory. For very large models, this could be inefficient. Consider using a generator or mapping approach if the Xpress API supports it.

4. Code Style:

  • linopy/model.py:1363-1370: Consider extracting the type annotation list initialization into a more readable format, possibly using a dataclass or named tuple for better maintainability.

📝 Minor Issues

1. Documentation:

  • linopy/model.py:1284-1288: The docstring mentions "This function requires that the model was solved with gurobi or xpress" but doesn't specify which termination conditions are expected. Consider adding this clarification.

2. Code Organization:

  • linopy/model.py:1345-1395: The method is quite long. Consider extracting the IIS data retrieval logic into a separate helper method for better readability.

🔧 Suggestions

1. Type Safety Enhancement:

# Instead of:
miisrow: list[Any] = []

# Consider:
miisrow: list[Any] = []  # xpress.constraint objects

2. Error Handling Enhancement:

# Around line 1389:
try:
    idx = all_constraints.index(constraint_obj)
    labels.add(idx)
except ValueError:
    # Log or handle the case where constraint is not found
    logger.debug(f"Constraint {constraint_obj} not found in model constraints")

3. Memory Optimization:

# Consider creating a constraint index mapping once:
constraint_to_index = {constraint: idx for idx, constraint in enumerate(solver_model.getConstraint())}

📊 Overall Assessment

This is a solid implementation that successfully extends IIS functionality to Xpress solver. The code follows established patterns in the codebase, maintains backward compatibility, and includes comprehensive testing. The mathematical approach is sound and the performance impact is minimal.

Score: 8.5/10

The implementation is production-ready with the current code. The suggested improvements would enhance maintainability and type safety but are not blocking issues.


FabianHofmann and others added 3 commits July 10, 2025 21:01
- Extract IIS constraint extraction logic into separate helper method for better readability
- Optimize memory usage by using constraint-to-index mapping instead of repeated list.index() calls
- Add more detailed type annotations and documentation in _extract_iis_constraints method
- Improve docstring clarity for compute_infeasibilities method
- Add explanatory comments for constraint mapping edge cases

Addresses feedback from automated code review focusing on:
- Code organization and maintainability
- Performance optimization for large models
- Better type safety and documentation

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add return type annotations to all test fixture methods (-> Model)
- Add parameter and return type annotations to all test methods (-> None)
- Fix mypy no-untyped-def errors for comprehensive type safety

Resolves mypy type checking issues in the new test file while maintaining
full test functionality and pytest compatibility.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…orted solvers

- Fix CI test failures by raising NotImplementedError instead of ValueError for unsupported solvers
- Improve logic flow to check solver support first, then solver model availability
- Add better test coverage for different error scenarios (unsolved models vs missing solver models)
- Maintain backward compatibility for supported solvers (gurobi/xpress)

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@FabianHofmann FabianHofmann merged commit f9e3a5e into master Jul 22, 2025
21 checks passed
@FabianHofmann FabianHofmann deleted the xpress-iis branch July 22, 2025 15:23
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.

1 participant