-
Notifications
You must be signed in to change notification settings - Fork 53
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 evaluation method for arrays of TaylorN #364
Conversation
Rebased to current |
The proposed method works as follows: x = set_variables("x", order=2, numvars=4)
# helper function to fill TaylorN with random coefficients
function radntn!(y)
for k in eachindex(y)
for l in eachindex(y[k])
y[k][l] = randn()
end
end
nothing
end
y = zero(x[1])
radntn!(y)
n = 10
v = [zero(x[1]) for _ in 1:n] # array of TaylorN to evaluate
r = [zero(x[1]) for _ in 1:n] # output vector
radntn!.(v)
x1 = randn(4) .+ x # TaylorN variables to substitute into `v`
@time TaylorSeries.evaluate!(v, (x1...,), r) # new evaluate! method
@time r2 = evaluate.(v, Ref(x1)) # current mechanism (could also do it via map!)
r == r2 # true @LuEdRaMo can you check if this works faster than the current workaround while maintaining essentially the same results? |
One thing to note is that as currently proposed the new |
Also, currently this |
In my # TS: master
43.177703 seconds (458.16 M allocations: 127.817 GiB, 35.71% gc time)
# TS: jp/evaluate-tn
21.322079 seconds (209.04 M allocations: 25.796 GiB, 18.86% gc time) However, |
This looks very promising! Regarding TaylorIntegration, I hope to find time and work on that this afternoon... |
How many times (approx) are you calling the
For this, you can try to use the following setup in your environment: TaylorSeries.jl branch Should work in principle, but do let me know otherwise! |
I opened PerezHz/TaylorIntegration.jl#194 to do the corresponding updates coming from #361, maybe we can join efforts there! |
Thanks for opening that PR. I'll have a look and try to ammend |
Actually tests are not passing in PerezHz/TaylorIntegration.jl#194 so use with caution; I'm having a look now 🤔 |
Clever |
A small precision/update on this: I updated |
This is now ready for review @lbenet |
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.
Thanks a lot! LGTM! I'm merging this and tagging the new release.
Merged! |
This is still WIP, butthe idea here is to improve allocations when substituting a tuple ofTaylorN
s (let's call itvals
) into each element of an arrayarr
ofTaylorN
. So far I haven't managed to get rid of all allocations, but instead the number of allocations is constant, no matter the size ofarr
.EDIT: no longer WIP, now ready for review.