-
Notifications
You must be signed in to change notification settings - Fork 327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Trapping part 2/3 #493
Merged
Merged
Trapping part 2/3 #493
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
This is much faster than fitting the features, and can be used by trapping to replace some constraint index calculations. Also: Add docstring/type annotations Rename a variable so it doesn't shadow newly-imported name
There are several major differences from the version in example 8: * Coupled with PolynomialLibrary logic using PolynomialLibrary.powers_. Previously, this coupling was implicit and actually tied to the specific order in Example 8, not the order users would expect. * Instead of handling pre-flattened 2D array, constraints are built with target and feature axes separately, letting the caller handle formatting the constraints as "target" or "feature". This gives us flexibility to later clean up the constraint logic outside this function without needing to return here and removes the need for striding calculations like: n_tgts * (n_tgts + k - 1) + i + n_tgts * int(j * (2 * n_tgts - j - 3) / 2.0) Since constraints are built directly from the terms and exponents in the library, its much easier to read and debug the code. * Renamed "r" as "n_tgts", etc. Single-letter index variables are removed. When they occur in homogenous iterators, they are concatenated with what they're iterating (e.g. tgt_i, tgt_j) * Broke out the different kinds of constraints into helper functions to make it easier to test. This allows follow on work to auto-attach constraints to a problem.
(other than degree, which has to be 2)
Previously, TrappingSR3 could only use constraints passed to it, and only then a limited set of constraints. It also didn't apply the trapping constraints automatically, because constraints were required at __init__, and actually shaping them requires knowledge about the number of features, typically not known until fit (unless the user is a developer who knows how the feature libraries work internally 😉) WIP, Spawned issue #452
And simplify reorder_constraints
Read the fucking diff backport-to: constraint-order
Also: * (constrained_sr3)clean up creating cvxpy constraints * (constrained_sr3)Clone the cvxpy problem in case of except statements, because prob.solve() mutates prob in a mathematically significant way
Jacob-Stevens-Haas
force-pushed
the
trapping-plumes
branch
from
April 16, 2024 20:23
fe6bc56
to
a88a1f2
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #493 +/- ##
==========================================
- Coverage 94.81% 94.68% -0.14%
==========================================
Files 38 38
Lines 3996 4081 +85
==========================================
+ Hits 3789 3864 +75
- Misses 207 217 +10 ☔ View full report in Codecov by Sentry. |
jpcurbelo
pushed a commit
to jpcurbelo/pysindy_fork
that referenced
this pull request
Apr 30, 2024
Trapping part 2/3
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 is part 2/3, clarifying the way constraints are applied and allowing other constraints to be added to Trapping problems, by dispatching all to Constrainted SR3. It also solves some bugs in cvxpy
probs
. Part 3/3 will be the Mai's local trapping bit.Currently there is a small hack - constraints are added at initialization, but in order to add trapping constraints, we need to know the size of the problem, which are data-dependent parameters. Solution in the interim is to add arguments to init that begin with an underscore.