Issues with pytorch bindings in the case where structural_feasibility=False. #414
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.
This PR addresses the issue raised in #406.
Summary of Changes
I’ve added a test file under test/src/ to verify the correctness of the PyTorch bindings (with and without structural feasibility) by comparing them to the NumPy bindings.
The test only checks that gradients are returned, not that they are numerically correct (yet).
If needed, I can extend the test to compare PyTorch gradients to the NumPy ones.
For now, the test only covers the case where:
The goal was to test that both forward and backward passes work as expected in a simple, feasible QP setup.
Note on cvxpy.py Rename
To avoid a conflict with the official cvxpy Python library, I had to rename the file:
test/src/cvxpy.py → test/src/cvxpy_test.py
This is because I use the actual cvxpy library inside the new test file.
If the rename causes a problem for your codebase structure, I can:
Please do not merge this PR until we confirm the cvxpy.py rename is acceptable.
Known Limitations
This PR does not guarantee the bindings behave correctly in all edge cases.
I suspect there may still be minor bugs in cases such as:
For now, this PR ensures that forward/backward behavior is sound in the simple case I test.
For now, the PR gives an idea of the kind of issues we encounter in the bindings. These issues apparently only concern cases where structural_feasibility=False. I will likely continue the PR either next week or at the beginning of September to ensure everything is okay.