-
Notifications
You must be signed in to change notification settings - Fork 299
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
Better assy solver implementation #1063
Conversation
- scaling - minimal quat paramterization - dummy variables
- Different axis cost - Restructured remaining costs - Perturbed starting point
@lorenzncode not finished yet, but you could already take a look. |
Codecov Report
@@ Coverage Diff @@
## master #1063 +/- ##
==========================================
+ Coverage 96.32% 96.37% +0.04%
==========================================
Files 40 40
Lines 9427 9496 +69
Branches 1251 1256 +5
==========================================
+ Hits 9081 9152 +71
+ Misses 204 202 -2
Partials 142 142
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
What a big effort the new implementation is. I tested the constraints and did not find any significant problem.
|
Thanks @lorenzncode . I changed to extrinsic and now it is explicitly disallowed to have only one unary constraint (or only one constrained entity to be exact). |
"nlp_scaling_method": "none", | ||
"honor_original_bounds": "yes", | ||
"bound_relax_factor": 0, | ||
"print_level": 5, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The print output to stdout is useful. Perhaps an option can be exposed to reduce verbosity if the user chooses (not necessary for this PR can open a separate issue).
assy.solve(options={"verbose": 0})
(key "verbose", "verbosity", "print_level" or similar)
An options dict may be potentially useful to pass future options as well to control other algorithm settings or preferences (passed through to ConstraintSolver).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's open another issue for this. TBH I was thinking more in the direction of removing it in the near feature (once the solver is more established.
OK I think it is good enough now @lorenzncode @jmwright |
+1 to merge @adam-urbanczyk @lorenzncode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @adam-urbanczyk!
This PR implements a new assy solver based on Casadi (for autodiff) and IPOPT (actual optimizer, called via casadi).