-
Notifications
You must be signed in to change notification settings - Fork 10
Update the default value of theta in ROSolverOptions and update LM & LMTR accordingly #180
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
Conversation
a10d66a to
d34e765
Compare
|
I think we can merge now @dpo |
|
@MaxenceGollier In #178 (comment), we also talked abour replacing |
|
Done @dpo, the implementation of LMTR is confusing in my opinion, please review my changes before merging. |
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.
Much better. Thank @MaxenceGollier ! Just one final improvement.
|
Can we merge now ? @dpo |
|
Thank you @MaxenceGollier. Could you please rebase? All the workflows fail because some workflow dependencies are outdated. I just merged an update. Also, please open a PR to print ν instead of 1/ν in the other solvers. Thanks! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #180 +/- ##
==========================================
- Coverage 61.53% 58.22% -3.31%
==========================================
Files 11 14 +3
Lines 1292 1824 +532
==========================================
+ Hits 795 1062 +267
- Misses 497 762 +265 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
#178