Draft: change nlp_solver Oracles class to use C diffengine. #131
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Replace the pure-Python Oracles class with a new implementation that wraps the C-based C_problem class from dnlp_diff_engine. This provides automatic differentiation via the compiled C library instead of the Python-based jacobian/hessian computation. Key changes: - Replace Oracles class implementation in nlp_solver.py - Use lazy import to avoid circular dependency at module load time - Convert CSR sparse matrices from diff engine to COO format for solvers - Cache sparsity structures for jacobian and hessian Currently supported atoms: log, exp, sum, AddExpression, NegExpression, Promote. Other atoms (multiply, power, matmul, etc.) need to be added to the diff engine before all NLP tests pass. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Document all newly added Python bindings (power, trig, hyperbolic, matmul) - Update test status: 2/15 IPOPT tests now pass - Clarify blocking issues for remaining tests (index, norm, C bugs) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Benchmarks that have improved:
Benchmarks that have stayed the same: |
…alue extraction - Remove redundant objective_forward/constraint_forward calls from gradient(), jacobian(), and hessian() since NLP solvers guarantee call ordering - Replace matrix densification + zip iteration with direct COO data return for jacobian and hessian value extraction Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Updated implemented atoms list (index, reshape, sqrt, quad_over_lin, rel_entr) - Updated test results summary with current pass/fail status - Added known issues section (segfaults, bivariate matmul, rel_entr scalar) - Added missing atoms by priority Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Member
Author
|
the first commit cleans up the inefficient value retrieval for the jacobian and hessian. |
- reshape atom now fully implemented with Python binding (commit 81c5a12) - All 3 circle packing tests now PASS - test_clnlbeam now PASSES (previously segfaulted) - Memory issues with many constraints fixed - Only test_localization still failing (needs broadcast_to) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Full nlp_tests results (2025-01-14): - 259 passed, 20 failed, 64 skipped, 1 xfailed Remaining failures by missing atom: - broadcast_to: 5 tests (localization, broadcast, best_of) - Prod: 9 tests - MulExpression (bivariate matmul): 5 tests - rel_entr scalar variants: 1 test Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Examples: X @ Y (both Variables), cos(X) @ sin(Y) Currently supported: A @ f(x) and f(x) @ A where A is constant Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Added Prod to implemented atoms list - Updated test_prod.py results: 13 passing, 1 failing (axis param) - Updated summary: 267 passed, 12 failed (down from 20) Remaining failures: broadcast_to (5), Prod axis (1), MulExpression (5), rel_entr scalar (1) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create cvxpy/reductions/solvers/nlp_solvers/diff_engine/ package - Move C_problem wrapper and ATOM_CONVERTERS from dnlp-diff-engine - converters.py: Expression tree conversion from CVXPY atoms to C nodes - c_problem.py: C_problem class wrapping the C problem struct - Update nlp_solver.py to import from new location This separates CVXPY-specific glue code from the pure C autodiff library, eliminating circular dependencies and preparing dnlp-diff-engine for standalone PyPI distribution. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Merge license header and module docstring into single docstring to avoid "module level import not at top of file" errors. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…asting rule. This is now internally done in the diff engine
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR is currently in draft and will probably stay so for a while.
The purpose is to try out the C differentiation engine that @dance858 and I have been working on. This will allow us to seamlessly try out the many tests that we wrote for DNLP.