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

REF: simplify function factory #43

Open
albop opened this issue Sep 18, 2017 · 1 comment
Open

REF: simplify function factory #43

albop opened this issue Sep 18, 2017 · 1 comment

Comments

@albop
Copy link
Member

albop commented Sep 18, 2017

@sglyon: I am starting to like the FunctionFactory object but find it somewhat inconsistent (partly my fault of course.
Here is what we have, for flat arguments (resp group arguments):

Dolang.FunctionFactory{Array{Tuple{Symbol,Int64},1},Array{Symbol,1},Dict{Symbol,Expr},DataType}
    eqs → Expr[2]
        _foo__0_ = log(_a__0_) + _b__0_ / (_a_m1_ / (1 - _c__0_))
        _bar__0_ = _c__1_ + _u_ * _d__1_
    args → Tuple{Symbol,Int64}[6]
        (:a, -1)
        (:a, 0)
        (:b, 0)
        (:c, 0)
        (:c, 1)
        (:d, 1)
    params → Symbol[1]
        :u
    targets → Symbol[2]
        :_foo__0_
        :_bar__0_
    defs → Dict Symbol → Expr with 1 entries
        :x → a(0) / (1 - c(1))
    funname → :myfun
    dispatch → Dolang.SkipArg
    incidence → Dolang.IncidenceTable

(resp.

Dolang.FunctionFactory{DataStructures.OrderedDict{Symbol,Array{Tuple{Symbol,Int64},1}},Array{Symbol,1},Dict{Symbol,Expr},DataType}
    eqs → Expr[2]
        _foo__0_ = log(_a__0_) + _b__0_ / (_a_m1_ / (1 - _c__0_))
        _bar__0_ = _c__1_ + _u_ * _d__1_
    args → DataStructures.OrderedDict{Symbol,Array{Tuple{Symbol,Int64},1}} with 3 entries:
      :x => Tuple{Symbol,Int64}[(:a, -1)]
      :y => Tuple{Symbol,Int64}[(:a, 0), (:b, 0), (:c, 0)]
      :z => Tuple{Symbol,Int64}[(:c, 1), (:d, 1)]
    params → Symbol[1]
        :u
    targets → Symbol[2]
        :_foo__0_
        :_bar__0_
    defs → Dict Symbol → Expr with 1 entries
        :z → a(0) / (1 - c(1))
    funname → :myfun
    dispatch → Dolang.SkipArg
    incidence → Dolang.IncidenceTable

)

It looks to me that some of this content is not useful to write functions and require further processing.
I propose the following simplifications,
1/ all symbols and equations are normalized (in args list and in defs)
2/ params disappears
3/ definitions are preprocessed and stored in the right order so they can be computed sequentially (not clear in the examples above)
4/ incidence is replaced by an incidence table without any notion of time.

The first example would become (only the changing fields):

    args → DataStructures.OrderedDict{Symbol,Array{Symbol,1}} 
        :x=> [ :_a_m1_, :_a__0_, :_b__0_, :_c__0_, :_c__1_, :_d__1_]
        :p=> [:u]
    defs → Dict Symbol → Expr with 1 entries
        :_z__0_ => :(_a__0_/ (1 - _c__1_))
        :_w__0_ => :(1/_x__0_)

and the second one:

    args → DataStructures.OrderedDict{Symbol,Array{Symbol,1}} 
          :x => Symbol[:_a__m1_]
          :y => Symbol[:_a__0_, :_b__0_, :_c__0_]
          :z => Symbol[:_b__1_, :_c__1_]
          :p => Symbol[:u]
    defs → Dict Symbol → Expr with 1 entries
        :_z__0_ => :(_a__0_/ (1 - _c__1_))

I see a few advantages to this change:
1/ it allows to experiment much more easily with other types of function generation (a basic function is super easy to write, since one just needs to list all definitions, then all equations (and unpack/pack the argument vectors)
2/ it is quite indifferent to where the normalized symbols come from. They can be simple timed variables or come from more complex indexing schemes ( :_∂x_i_∂y_j )

I don't see any disadvantage.

  • For "flat arguments" (the distinction kind of disappears), we can define f(x,p), f(Der{1}, x, p), f(Der{2}, x, p), with the convention that first argument is the one that get differentiated. So f(Der{2}, x, y, p), would be ∂2f/∂y2. This would of course work if there are no definitions or after they are substituted in the other equations.
  • For "grouped arguments" we could have selective differentiation like f(Der{0,:x,:z}, x,y,z,p) which would return f,∂f/∂x,∂f/∂z.

To smooth the transition, we could have both object coexist.
@sglyon : what do you think ?

@sglyon
Copy link
Member

sglyon commented Sep 19, 2017

Hey @albop thanks for taking the time to write this up.

I think it would be very nice to refactor the FunctionFactory code -- much of what was contained in there is a remnant from a time when I did things like equation verification using an instance of the type. Given that this is no longer done, we can definitely clean some things out.

I don't have the energy to carefully review the proposal right now and will be traveling tomorrow, but will try to find time soon to give a more thoughtful response. If I don't respond in the next couple of days, please remind me and I will make it happen.

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

No branches or pull requests

2 participants