Lost functionality with new way of handling init argument?

[papamarkou]

Hi @fredo-dedup, there is sth that stopped working with the recent change of interface for the init input argument of rdiff(). Say that I have an expression with two variables x and v, of types Vector{Float64} and Vector{Any}. This won't work anymore:

using ReverseDiffSource
rdiff(:(v[1]*x+v[2]), x=Vector{Float64}, v=Any[Matrix{Float64}, Vector{Float64}], ignore=:v)

In the past, it was possible to do sth along the lines:

using ReverseDiffSource
rdiff(:(v[1]*x+v[2]), x=ones(2), v=Any[ones(2, 2), ones(2)], ignore=:v)

Is there a solution that wouldn't require breaking the existing interface in Klara?

[fredo-dedup]

You are right that using types introduces limitations in the information that can be given to the parser.
I can see 2 valid ways to make this work:

• define a type

type AA
    X::Matrix{Float64}
    y::Vector{Float64}
end

rdiff(:(sum(v.X * x + v.y)), 
      x=Vector{Float64}, 
      v=AA, 
      ignore=:v)

• separate variables

rdiff(:(sum(X * x + y)), 
      x=Vector{Float64}, 
      X=Matrix{Float64},
      y=Vector{Float64},
      ignore=[:X,:y])

Does that solve your problem or is it still problematic for your use case ?

[papamarkou]

This one is a little more complicated. The interface relies on taking a second arbitrary user-defined argument of type Vector, so the first option to define a type is precluded. The latter in theory would work, but it would break the whole interface of Klara, in the sense that all additional arguments are passed as a single vector. "Unrolling" them as separate arguments would require rethinking the whole interface. Is there a way that we can restore the previous functionality of ReverseDiffSource, and replace the numeric values in the vector of the second argument by their respective types, thus allowing a vector of types rather than a single type?

[papamarkou]

@fredo-dedup I was thinking about this issue, and understand of course that using types limits the information that can be passed to the parser. Along these lines, I wanted to ask if it is possible to revert this breaking change; if we have to choose between one interface which limits the previously available functionality and an interface like the old one with richer functionality, I would prefer the latter, if this is ok with you.

[fredo-dedup]

I am a bit reluctant to revert the change since it brings a lot of advantages both internally : differentiation code generation is faster and smaller, and externally : user does not have to provide actual values but only their type. Maintaining both possibilities (values or types) is also not very satisfactory since I still lose on the internal side of things.
Perhaps I could give you a hand on finding a solution in Klara ? Can you point me to the place where the problem appears ?

[papamarkou]

Thanks @fredo-dedup. I am afraid changing Klara's API is far from trivial, because there are so many inter-related components that must by satisfied concurrently (ReverseDiffSource and ForwardDiff compatibility being one), plus propagating a potential API change throughout the package is not the easiest task due to the extensive amount of code involved (besides, this would lead to yet another breakage of interface for users, as the interface at some point needs to settle to let the users make use of the package without frustrating them).

In a nutshell, the main issue is the following; let's say we want to call the logtarget(x) function, where x is the value of the parameter that follows this logtarget. This is how we used to call it. However, we may want to pass on additional data or values of other parameters, which can be done by calling logtarget(x, v), where v is a vector of type Vector{Any} holding any additional data or parameter values. This vector v can be passed to all parametes, it has the same ordering. The only obvious alternative is to call, logtarget(x, v1, v2, ..., vn) but that will result in trying to pass variable number of arguments, as in logtarget(x, ...), which might or might not work, would have to think about it... worst case senario, if we can't find a solution, we can consider the possibility of not having ReverseDiffSource as a dependency in Klara (if nothing else works)...

[papamarkou]

@fredo-dedup please excuse my persistence for trying to find a solution for this, but it is critical for me to fix it; firstly it breaks the relevant functionality of Klara, and secondly I need it for a project of my own (I submitted a paper for review to a journal, which was using ReverseDiffSource from Klara and I truly need this functionality desperately back in order to be able to run follow up simulations related to the manuscript). Is there any chance you can help me to create a clone of your repo with the appropriate code that allows the older richer functionality, associated with this issue, be invoked? If you could give me some instructions as to which are the relevant commits I need to invert in a cloned repo of ReverseDiffSource, I would be indebted; apologies in advance for the urgency of my query.

[fredo-dedup]

Going back to v0.2.3 of ReverseDiffSource should do it, no ? Can't you change the require file of Klara to have an upper limit at 0.2.3 ?
The new interface was implemented in v0.3.0

[papamarkou]

Brilliant, I will check this out in the next couple of days. Is ReverseDiffSource 0.2.3 compatible with Julia v0.5?

[papamarkou]

Hi @fredo-dedup, I tried what you suggested, used version 0.2.3 of ReverseDiffSource with the latest version of Klara (0.7.0). It doesn't work still, the errors remain. Moreover, in relation to my question above, is v0.2.3 of ReverseDiffSouce meant to run with Julia v0.5 or Julia v0.4? Is there a way ReverseDiffSource could possibly provide a solution so that the relevant functionality of Klara relying on your package remains operational? Can you help maintain ReverseDiffSource as an integrated component of Klara, which is much desired and in some instances needed due to packages/projects depending on your package? Thanks for your attention to this issue.

[fredo-dedup]

v0.2.3 is julia v0.4 only (it supported 0.5 pre release versions but not the final one). But i think that there must have been an operational version of klara that was working with this version of reversediffsource (what was the version of klara at the time of the release of v0.2.3 of reversediffsource ?). This version at least should work no ?
For my part I have been thinking about bringing back the old interface alongside the new one, but had very little time to translate that in a delivered solution, sorry about that. This will be one of my top issues to solve.

[papamarkou]

Yes, @fredo-dedup, I will try to choose the right commits for ReverseDiffSource and Klara to have them working again together for the needs of my own project.

I am very thankful that you are thinking of bringing back the old interface alongside the new one, as this will ensure longer term integration between ReverseDiffSource and Klara. The timeline for such sustainable delivered solution is not an issue at all (I can settle with the temporary solution you proposed for my own needs for now). It is more than enough to know that you are planning to tackle this matter and that it is in your high priority list, thank you very much for this.