-
-
Notifications
You must be signed in to change notification settings - Fork 49
RFC: new adaptive (and fixed step) Runge Kutta solver #68
Conversation
bf7d288
to
a971878
Compare
# The advantage of each having its own type, makes it possible to have | ||
# specialized methods for a particular tablau | ||
immutable TableauRKExplicit{Name, S, T} <: Tableau{Name, S, T} | ||
order::(Int...) # the order of the methods |
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 will break on 0.4 with the newly introduced tuple changes; at the moment, it should probably be order::(@compat(Tuple{Vararg{Int}}))
but I'm unsure on how this works in this context; the outer parenthesis might throw this off.
It will work on 0.3 as is, though :)
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.
It sure failed on Travis!
Actually, this doesn't solve the problem that #60 addresses, since you've effectively converted all the tables to |
Ah, yes, you're right. Another step is needed. But my idea was that the parameter bt_rk45_coef = ( [ 0 0 0 0 0 0
1//4 0 0 0 0 0
3//32 9//32 0 0 0 0
1932//2197 -7200//2197 7296//2197 0 0 0
439//216 -8 3680//513 -845//4104 0 0
-8//27 2 -3544//2565 1859//4104 -11//40 0 ],
[25//216 0 1408//2565 2197//4104 -1//5 0
16//135 0 6656//12825 28561//56430 -9//50 2//55],
[0, 1//4, 3//8, 12//13, 1, 1//2])
bt_rk45 = TableauRKExplicit(:fehlberg,(4,5),Float64, bt_rk45_coef...)
bt_rk45_f32 = TableauRKExplicit(:fehlberg,(4,5),Float32, bt_rk45_coef...) |
The problem is, that the type of the solution is determined by the type of the time-step (most likely float), the type of the tableau coefficients and the type of Nice work BTW! |
My idea was that 99% of users will work with Float64. For the other 1%, they can instantiate something like And thanks. But, I just had a look at performance with IVPtestSuite and it's atrocious, so still lots to be done... |
|
||
## Tableaus for explicit methods | ||
# Fixed step: | ||
bt_feuler = TableauRKExplicit(:feuler,(1,), Float64, |
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.
const
?
The reason to store the coefficients as rational is that they are the least invasive when promoting with other number types. Then, we can do something conceptually equivalent to TTime = eltype(tspan)
TRHS = typeof(F(0, #= whatever args we need =#))
TTableau = typeof(one(TTime) * one(TRHS) * one(Rational{Int}) and then (possibly) convert the coefficients. This way,
|
Sorry for nitpicking but I am in the 1% not using |
@mauro3 The reason this won't work is that it's impossible for us to prepare in this way for the case where someone has a self-created number type which doesn't promote cleanly with @pwl I don't think the current version of this PR works like that - and it might turn out to be out of scope for this PR to make it work like that. However, I do think it's valuable enough that we should solve this at some point. We'll see where this leads to and take it from there, I guess. However important generics and sensible promotion rules are, I think the main merit of this PR is to provide a white-room Runge-Kutta implementation, allowing us to greatly simplify the license of this package. That's no small thing, indeed! :) |
I just wanted to ensure that the 1% is well represented. :) And I already have my own working fork so there is no hurry in implementing this feature. Don't get me wrong, I see this PR as a remarkable piece of work. |
After fixing a bug in the step control, this now works. Here the performance tests from IVPTestSuites.jl: The new solvers ( All solvers new solvers a factor 2-5 faster. And for the fixed step solver: The solver is significantly better than the old ones. This is a bit surprising as the |
@tlycken, yes I agree, that was the idea behind the little code example above, in |
Very nice :-)
Well, in the old code many copies of the solution are made, maybe this explains the difference? Partly, this was necessary to make the code work with user-defined types. At the moment, |
dof = length(y0) | ||
tsteps = length(tspan) | ||
ys = Array(T, dof, tsteps) | ||
ys[:,1] = y0' |
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.
Its probably better to use transpose
here. But what happens if y0
is not an array?
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.
Changed to .'
(I think .'
is sugar for transpose
, right?)
@acroy: we should add tests for non-abstractarrays if we want to support them. But what is the use case? |
@acroy: and no, at the moment it doesn't work quite work for those general cases. It also stores the ys in a Matrix internally. I can update that. Related, I find the current interface returning a Vector of something quite annoying. I can see that for the general case that is needed. But I always have to |
@acroy: before updating the PR to allow for more general datatypes, a few questions: What is the "interface" a |
I agree that the typical use case will involve scalars or dense vectors, but there might be good reasons to have a user-defined type, which doesn't support the full array interface. For example, in JuliaQuantum/QuBase.jl we have
Yeah, we discussed this when we put together the API specification (probably in #20). I think one point was, that one wants to use |
@mauro3: I think it is just |
I haven't done any profiling, but if memory management is the bottleneck here, we could very probably use the iterator versions (#49) to let the user take care of it in cases where they want to:
If we choose that path (and there is a clean-enough way to do it for the user, so we can show it off in examples), we can move that issue out of scope for this PR :) |
I'm trying to adapt the solvers to the preferred API and internals but I find it quite painful and I'm wondering whether this in not a case of over-engineering? There are a few types involved:
Anyway, I wonder whether there is any need to support other containers than AbstractArrays? And in particular whether internal, temporary storage arrays can be just The reason I am wondering these things is that I would like to see an API developed, similar to Sundials, where all storage can be preallocated and functions do in-place updated. I don't see how this is compatible with above internals. I don't think an iterator method would work, at least not if it is not in-place. Sorry for the rant. I'll ponder this a bit longer... |
The assumption that On the other hand the in-place updates are a must have feature if we want good performance in the iterator version and I don't see how this could be realized with custom types of |
And just a comment, in implicit methods My opinion is that types other than vectors would be very impractical to work with inside ODE.jl for reasons already mentioned and I think we should refrain from implementing API this way. Anyway, with iterators in place one can easily produce wrappers to generate user defined types from the array returned at each step via |
I removed @berceanu: I definitely cannot reproduce your error. I ran your topo-condensate code and it works for me (well the |
I did
and using ODE still gives the same error. Am I missing something? On 05/22/2015 10:53 AM, Mauro wrote:
|
I did those steps as well, just to be sure, and still no error here. I also tried with a build of the realease-0.3 branch with:
with no error. What operating system are you on? You compiled from source with the If you got a 0.4 version around, can you try that? |
OK so I did a On 05/22/2015 01:31 PM, Mauro wrote:
|
Cool, thanks! I think that must have been the new Tuple types which were not included in the old compat. Are those assertion errors I got running your code anything to worry about? |
Well, it looks like with the new ode45, for my test case I get the same On 05/22/2015 02:14 PM, Mauro wrote:
|
Perhaps add a minimum version to REQUIRES for Compat.jl? |
Features: - adaptive solvers with orders: 21, 45, 54, 78 - fixed step solvers with order: 1-4 - faster for Vector ODE states - a bit slower for scalar ODE states - MIT licensed - added support for keyword `points` - more general framework for coefficients tableaus - more general framework for determining the types of the computation TODO after merging this PR: - make solving scalar-like states faster again - use more general tableau-framework for the other solvers too Bumped required Julia to 0.3, added Compat 0.4.1 requirement.
Ok. I just tagged a new version for ODE.jl/master. From my side we can merge this. @pwl also gave +1 and there were no objections. Let's do it then. |
New adaptive (and fixed step) Runge Kutta solvers.
Merged by popular demand :-) |
:-) |
Great! Tanks @mauro3! |
@mauro3 : I knew I forgot something: could you please update LICENSE.md? |
It's here #73. Let me know what you think. |
💯 👍 ❗ Great job landing this! |
As promised in #61, here a RK implementation sans license restrictions. It is a bit longer than the current code but has some more features as well.
There is probably a bug lurking somewhere as it is less accurate than the current solvers.Extra features:
Also, this "solves" what was tried in PR WIP: Make coefficients Rational for better generics #60: the method will run using the type of number used for the coefficients.Improvement in ODE.jl to have
hinit
returntdir
as well.TODO:
doc update(there is no doc yet)This touches on issue #9 and fixes #25 (although that was fixed already, no?).
Extra features, maybe in this PR, maybe later:
iterator protocolmaybe implement dense output in terms ofPolynomials.jl
, root finding.