EconForge / Dolo.jl

Economic modeling in Julia
Other
58 stars 29 forks source link

Change default array orientation #107

Closed albop closed 6 years ago

albop commented 7 years ago

Currently, a list of points of the state space is represented by an array of dimension, Nxd where d is the number of dimensions and N the number of points. The corresponding object is declared as Matrix{Float64}.

This comes with a sizeable penalty gain. I propose the following transition plan to switch array orientation everywhere:

1/ Ensure that all arrays returned by functions are AxisArrays, that can be used without knowing orientation 2/ Adjust DecisionRules.* and all internal functions operating on Matrix{Float64} to operate on alias VectorOfPoints{d} = Vector{SVector{d, Float64}} while keeping compatibility functions operating on matrices. Inlcuding size of the vector in the type should in principle offer additional performance gains for things like interpolations. 3/ Allow Dolang to either operate on ListOfPoints and/or on horizontally layed out arrays (either by a compile-time switch or with an additional option in the generated functions). 4/ Explain clearly in the docs what ListOfPOints is and promote its use, while removing all vertially layed out arrays. 5/ Optional: reintroduce matrices, but layed out in the other way round. They can always be converted seemlessly to ListOfPoints.

sglyon commented 7 years ago

I think this is a change that will help give us some performance boosts.

I'll comment number by number...

  1. Using axis arrays is a particularly clever/insightful proposal because then we can have optimal ordering within our routines but users don't need to even think about it.
  2. I think this should be pretty easy and will definitely make hooking up Interpolations.jl much easier
  3. What do you mean by horizontally layed out arrays? Do you mean d x N instead of N x d? I think the change to Dolang to support VectorOfPoints should be really easy. Once we have a clear idea of what we want it should definitely take less than an hour to incorporate them into Dolang.
  4. šŸ‘
  5. I think we can even use reinterpret to do the conversion in place (without allocations)
albop commented 7 years ago
  1. Yes, that's what I meant with seemless

On Mon, 14 Aug 2017, 17:07 Spencer Lyon, notifications@github.com wrote:

I think this is a change that will help give us some performance boosts.

I'll comment number by number...

  1. Using axis arrays is a particularly clever/insightful proposal because then we can have optimal ordering within our routines but users don't need to even think about it.
  2. I think this should be pretty easy and will definitely make hooking up Interpolations.jl much easier
  3. What do you mean by horizontally layed out arrays? Do you mean d x N instead of N x d? I think the change to Dolang to support VectorOfPoints should be really easy. Once we have a clear idea of what we want it should definitely take less than an hour to incorporate them into Dolang.
  4. šŸ‘
  5. I think we can even use reinterpret to do the conversion in place (without allocations)

ā€” You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/EconForge/Dolo.jl/issues/107#issuecomment-322310157, or mute the thread https://github.com/notifications/unsubscribe-auth/AAQ5KQJZND23Sb-1yVx_BwR0C_8Wp01Iks5sYLcbgaJpZM4O2z5R .

albop commented 7 years ago
  1. is a bit more problematic. I was thinking of the following modifications to Dolang (which would all keep current behaviour as default for a while):
    • make_methods(...., layout=:horizontal)
    • Dolo.transition(..., Horizontal) where Horizontal would be a specific type to make dispatch easy. That would be an additional element to the generator.
    • Dolo.transition(s::ListOfPoints, x::ListOfPoints, ...)

Disadvantage of the first solution is that we have top update all the model calls in dolo at once (feasible). Otherwise, it is probably the easiest one. I am not sure the complications introduced by 2 and 3 are really worthwile.