Make TrappingSR3 a subclass of ConstrainedSR3 #435
Merged
Conversation
This file contains 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
Change base class from SR3 to ConstrainedSR3
Also: Rename single-letter variable to meaningful
Document it as an attribute, however.
If False, code skips all optimization methods. It isn't used in any example, it's just a vestige of earlier work. The section commented "update w" doesn't modify any variables called "w"
Also removes tolerance specifications from backup optimization due to name changes In some versions of SCS, (e.g. 3.2.3), the change from max_iter to max_iters raises a ValueError instead of a TypeError
Goal is to make it clear when trapping is solving for coefficients and when it's
solving for the updated trapping center.
* Renamed "m" "trap_ctr".
* Extracts solving for coef_prev from solve_cvxpy_direct, renaming latter to make
its reduced role in solving for just the trapping center
* Removes unused args
The "failed to converge warning" was emitted whenever max iteration was reached, regardless of whether convergence was achieved. The previous test only was able to assert convergence warning because it was written to successfully converge after 1 iteration, with a max iteration of 1. Also, removed test_sr3_warn and incorporated into test_fit_warn, with which it was 95% redundant already. This also required setting some variables outside a loop in order to access them later - a required practice for mypy and other type checkers.
This allows them to be used by set_params, part of the scikit-learn API
…ping This makes the algorithm in code look like algorithm in paper
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #435 +/- ##
==========================================
+ Coverage 93.87% 94.31% +0.43%
==========================================
Files 37 37
Lines 3626 3554 -72
==========================================
- Hits 3404 3352 -52
+ Misses 222 202 -20 ☔ View full report in Codecov by Sentry. |
|
Preempted by #459 , converting back to draft. |
jpcurbelo
pushed a commit
to jpcurbelo/pysindy_fork
that referenced
this pull request
Apr 30, 2024
Make TrappingSR3 a subclass of ConstrainedSR3
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.
This PR simplifies TrappingSR3, reducing it by about 250 lines. It changes the base class to
ConstrainedSR3, deferring all calls to cvxpy to that superclass. It also makes the main algorithm clearer, so that its easier to see how different initialization arguments affect the choice of math involved (mostly because I couldn't quite understand this from the docstring). As a related effect, the_reducemethod more closely matches the algorithm in the paper.The plan is to eventually utilize superclass for combining and applying constraints, since that class is capable of doing both equality and inequality constraints, which will make it easier to include the constraints and helper functions of example 8 into trapping_sr3.py