Skip to content
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

Moved reltol and abstol to optional parameters in ode() #8

Merged
merged 2 commits into from
Mar 14, 2014

Conversation

pjpmarques
Copy link
Contributor

While using the simplified ode() function interface I ran into problems because the precision was not enough for running my simulation. I changed the code so that the tolerances can now be specified as optional parameters of the function. This doesn't break any code and I think it's common/useful enough to be of used.

I've also included a link to a ijulia notebook that is using this code. I think this would be useful to other users.

@ViralBShah
Copy link
Contributor

Cc: @stevengj @jiahao

@acroy
Copy link
Contributor

acroy commented Mar 14, 2014

@pjpmarques : Thanks for the PR! This is very much in line with our API discussions for ODE.jl. Would you mind to add the keywords also for dae?

@ivarne
Copy link
Contributor

ivarne commented Mar 14, 2014

This is an obvious change that makes Sundails closer to the API discussions, without breaking backwards compatibility (for now).

ivarne added a commit that referenced this pull request Mar 14, 2014
Moved reltol and abstol to optional parameters in ode()
@ivarne ivarne merged commit 4fce783 into SciML:master Mar 14, 2014
@acroy
Copy link
Contributor

acroy commented Mar 14, 2014

@ivarne : Would have been nice to get reltol and abstol in dae too ...

@ivarne
Copy link
Contributor

ivarne commented Mar 14, 2014

Yes, it would, but I did not think that was a reason to hold this PR. At least when I submit something to a unfamiliar project, it is much more likely that I submit a second one, if the first gets merged quick.

@ivarne
Copy link
Contributor

ivarne commented Mar 14, 2014

@acroy I added the keyword arguments in dae too.

@pjpmarques
Copy link
Contributor Author

Sorry guys, I should have done this. Thanks.

@ivarne
Copy link
Contributor

ivarne commented Mar 14, 2014

Nothing to be sorry about. This change was almost a year overdue.

@acroy
Copy link
Contributor

acroy commented Mar 14, 2014

Thanks to both of you :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants