-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
Proposed solution to bug#745 #766
Conversation
The comment edit needs to be performed in SciMLBase and other solver packages.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #766 +/- ##
==========================================
- Coverage 68.51% 0.06% -68.46%
==========================================
Files 22 22
Lines 1607 1613 +6
==========================================
- Hits 1101 1 -1100
- Misses 506 1612 +1106 ☔ View full report in Codecov by Sentry. |
Updated convert function in BBO.
Two functions and a dictionary were added for RetCode handling.
Removed redundant Logging of RetCode types.
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.
This approach is pretty neat, I like that you decided to unify the handling. We should use this for all such conversions to retcode
in all the wrapper packages.
Changed convert function name to deduce_retcode and the deduce_retcode function to deduce_retcode_from_string.
Changed the convert function to deduce_retcode for retcode generation.
Retcode function for symbol and string separated.
In the deduce retcode function no need to pass RetCode::T.
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.
Looks good now. Can you also use it for some other solvers
Added retcode handling.
Updated the stop_reason_map for the lbfgsb conditions.
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Add any other context about the problem here.