sisl
Ind2x mvec
It seems that pre-specifying the vector size using MVector from StaticArrays helps to significantly improve the performance of ind2x
Could potentially be a breaking change if users were relying on the return type of ind2x to be Array rather than AbstractArray
Coverage decreased (-0.07%) to 95.726% when pulling 16403a73d0e2f4340d783fb15ccd83c6489672c1 on mattuntergassmair:ind2x_mvec into 5dcb4586e769e53c07eac71ad1f6c448e6402702 on sisl:master.
I like using StaticArrays! Shouldn't you be able to get it down to 0 allocs though? You have only reduced it to 1000000 from 2000000
I like using StaticArrays! Shouldn't you be able to get it down to 0 allocs though? You have only reduced it to 1000000 from 2000000
As for the allocations, I'm running the function benchmark_ind2x() with default parameters n_dims = 6, n_points_per_dim = 10, leading to 10^6 points. The benchmark function then calls ind2x() 10^6=1000000 times. So I think the allocs are as expected (as opposed to ind2x!() which should be non-allocating). I think what was happening earlier was that there was a copy somewhere leading to 2 x 10^6 allocs.
Could potentially be a breaking change if users were relying on the return type of
ind2xto beArrayrather thanAbstractArray
I think this is fine. If people are relying on a concrete array type, I will not feel sorry if their code gets broken :)
Shouldn't we expect a similar speed up by replacing those: https://github.com/sisl/GridInterpolations.jl/blob/master/src/GridInterpolations.jl#L36 by mutable statically sized arrays?
Shouldn't we expect a similar speed up by replacing those: https://github.com/sisl/GridInterpolations.jl/blob/master/src/GridInterpolations.jl#L36 by mutable statically sized arrays?
True, we could probly fix the size of most vectors that are part of the RectangleGrid struct and that should help with allocation and performance.
I could look into it but might need some help since I don't feel I understand the code base sufficiently well. Anyone want to team up on this (for a different PR)?
Those allocations are in the constructor, so I don't think it will make too much difference because they are only done once when the object is constructed.
On Fri, Mar 6, 2020 at 4:29 PM Maxime [email protected] wrote:
Shouldn't we expect a similar speed up by replacing those:
https://github.com/sisl/GridInterpolations.jl/blob/master/src/GridInterpolations.jl#L36 by mutable statically sized arrays?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sisl/GridInterpolations.jl/pull/32?email_source=notifications&email_token=ABALI26NZBQ4T2K4OVNV5OTRGGBODA5CNFSM4LCUEXH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEODFVIY#issuecomment-596007587, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABALI27K3ONJYOXWJJKLJTLRGGBODANCNFSM4LCUEXHQ .
Those allocations are in the constructor, so I don't think it will make too much difference because they are only done once when the object is constructed. … On Fri, Mar 6, 2020 at 4:29 PM Maxime @.***> wrote: Shouldn't we expect a similar speed up by replacing those: https://github.com/sisl/GridInterpolations.jl/blob/master/src/GridInterpolations.jl#L36 by mutable statically sized arrays? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#32?email_source=notifications&email_token=ABALI26NZBQ4T2K4OVNV5OTRGGBODA5CNFSM4LCUEXH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEODFVIY#issuecomment-596007587>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABALI27K3ONJYOXWJJKLJTLRGGBODANCNFSM4LCUEXHQ .
Agreed that it may not make a difference in the constructor itself, but we could make the member variables static in size, maybe the fixed size helps with memory optimization. I can give it a shot and report back.
As for the allocations, I'm running the function
benchmark_ind2x()with default parametersn_dims = 6, n_points_per_dim = 10, leading to 10^6 points. The benchmark function then callsind2x()10^6=1000000 times. So I think the allocs are as expected (as opposed toind2x!()which should be non-allocating). I think what was happening earlier was that there was a copy somewhere leading to 2 x 10^6 allocs.
@MVector zeros(D) Allocates. We should really do this with SVectors. Right now we are getting a 2x speedup with mvectors. I bet we will get a 10 or 100x speedup with svectors
For a broader utilization of MVector for the interpolation code, one thing to keep in mind is that if the size is larger than 100 we should expect decrease in performance according to the StaticArrays.jl readme. Since the size is given by 2^num_dimensions, it grows pretty fast.