-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add checking of keywords to make solve more robust, fix #166 #30
Conversation
Codecov Report
@@ Coverage Diff @@
## master #30 +/- ##
==========================================
+ Coverage 86.29% 86.43% +0.13%
==========================================
Files 4 4
Lines 197 199 +2
==========================================
+ Hits 170 172 +2
Misses 27 27
Continue to review full report at Codecov.
|
src/common.jl
Outdated
@@ -4,6 +4,8 @@ function solve{uType,tType,isinplace}( | |||
prob::AbstractODEProblem{uType,tType,isinplace}, | |||
alg::LSODAAlgorithm, | |||
timeseries=[],ts=[],ks=[]; | |||
|
|||
dense=false, verbose=true, |
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.
dense shouldn't be treated differently.
2 similar comments
Nice work. Note @rveltz that this actually requires a DiffEqBase tag, but it's already to merge earlier since instead of throwing a warning on an ignored keyword argument, pre-tag it will throw an error. We chose to go with warnings because technically the solution is still correct, but the user should know that some of what they think might be tweaked isn't actually tweaked. This is a clean solution that handles all of the keyword args. |
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.
Add dtmin
and dtmax
to the warnings list
Add dtmin, dtmax to the list of checked keywords
OK, dtmin/dtmax added |
No description provided.