FastDifferentiation v0.4.3 and newer break usage of make_Expr

[duetosymmetry]

Installing with FastDifferentiation v0.4.2 works. When installing with v0.4.3 or v0.4.4, I get the following error when precompiling:

ERROR: LoadError: MethodError: no method matching make_Expr(::StaticArraysCore.SVector{1, FastDifferentiation.Node}, ::Vector{FastDifferentiation.Node}, ::Bool, ::Bool)
The function `make_Expr` exists, but no method is defined for this combination of argument types.

Closest candidates are:
  make_Expr(::AbstractArray{T}, ::AbstractVector...; in_place, init_with_zeros) where T<:FastDifferentiation.Node
   @ FastDifferentiation ~/.julia/packages/FastDifferentiation/7my9Z/src/CodeGeneration.jl:221
  make_Expr(::SparseArrays.SparseMatrixCSC{T, Ti}, ::AbstractVector...; in_place, init_with_zeros) where {T<:FastDifferentiation.Node, Ti}
   @ FastDifferentiation ~/.julia/packages/FastDifferentiation/7my9Z/src/CodeGeneration.jl:306

The code in question is this block:

PostNewtonian.jl/src/pn_expressions/binding_energy.jl

Lines 169 to 186 in 7fda37f

	fdpnsystem = FDPNSystem(FloatType, PNOrder)
	# FD expects a single vector of variables, so we concatenate the state vector with the
	# two tidal-coupling parameters
	vars = FastDifferentiation.Node[fdpnsystem.state; Λ₁(fdpnsystem); Λ₂(fdpnsystem)]
	# Now we evaluate 𝓔 using the FD variables. This will expand all derived variables in
	# terms of the fundamental variables, but FD will take care of evaluating those
	# efficiently via common subexpression elimination (CSE).
	𝓔formula = 𝓔(fdpnsystem; pn_expansion_reducer=Val(PNExpansionReducer))
	# Now we take the derivative of 𝓔 with respect to v.
	𝓔′ = SVector(FastDifferentiation.derivative(𝓔formula, v(fdpnsystem)))
	# Turn that into an Expr (FD insists on making it a function)
	in_place = true
	init_with_zeros = false
	𝓔′expr = FastDifferentiation.make_Expr(𝓔′, vars, in_place, init_with_zeros)

I suspect the reason for this breakage was the following change on line 160 of src/CodeGeneration.jl in FD from v0.4.2...v0.4.3:

brianguenter/FastDifferentiation.jl@f5ae992#diff-b2b9a7d4664620d12817fc24b95f5532ea8a28744562bfce672174aba3ecb2b7R159-R161

which I quote here:

-function make_Expr(func_array::AbstractArray{T}, input_variables::AbstractVector{S}, in_place::Bool, init_with_zeros::Bool) where {T<:Node,S<:Node}
+function make_Expr(func_array::AbstractArray{T}, input_variables::AbstractVector...; in_place::Bool=false, init_with_zeros::Bool=true) where {T<:Node}

But I don't really know why this type signature isn't matching what's in binding_energy.jl (and separation.jl).

[duetosymmetry]

Ok now that I tried to look at it more carefully, I realize why it wasn't matching. Since AbstractVector... is trying to slurp all unnamed arguments, in_place and init_with_zeros had to become kwargs. So wee need to instead call

𝓔′expr = FastDifferentiation.make_Expr(𝓔′, vars; in_place, init_with_zeros)

or maybe it would work with earlier and later version to write

𝓔′expr = FastDifferentiation.make_Expr(𝓔′, vars, in_place=in_place, init_with_zeros=init_with_zeros)

Making this change results in an error later, when MacroTools.@capture tries to capture the @inbounds block. [An unrelated issue is that $pnsystem doesn't work in the throw expression; I'm guessing that's supposed to be $fdpnsystem instead?]

[duetosymmetry]

Ping @moble did you have any time to try to figure this out?

[moble]

Okay, I've realized the problem: I accidentally permitted newer versions of FastDifferentiation with an update to Quaternionic. Something broke involving FastDifferentiation and conditionals in v0.4, and is still broken, so it really shouldn't be used yet. (I was testing if it worked, but evidently forgot to change the compat bound back after I found that it didn't work.) I've submitted a PR to yank the bad version of Quaternionic. So when that merges installing PostNewtonian won't even try to install FastDifferentiation v0.4.*.

So until I fix the conditional issue with Quaternionic, this issue is moot. As for your PR, while my yank is still pending, I think it makes sense to just restrict PostNewtonian's compat bound to <0.4 as well.

[duetosymmetry]

@moble On my branch FD_up, I found how to make PostNewtonian.jl work with the latest version of FastDifferentiation.jl. However our Macro manipulation is pretty brittle, and relies a bunch on the internals of FastDifferentiation... I don't know the "right" way to fix this. I did not test what I have on that branch with any other versions of FastDifferentiation except the latest. I hope you know how to make this more robust — or, barring that, we have to pin the version of FastDifferentiation to the current one with this fix.

[duetosymmetry]

FYI I think it's a little brittle to hard-code the FD internal variable names result and input_variables1, but we can get these from MacroTools.splitdef(f′expr)[:args] elements 1 and 2.