Support for `&&`, `||`, and `isequal`

[moble]

Now that x==y returns a Node instead of a Bool, there are a lot of code paths that would benefit from being able to also use (x==y) && (a==b) or (x==y) || (a==b). At the moment, either of these will produce

ERROR: TypeError: non-boolean (FastDifferentiation.Node) used in boolean context

EDIT: isequal would also be good to have.

[brianguenter]

I agree this should be done. This requires a change to the code generation in make_function. There is already code there to handle the if...else case but the logic is slightly different for && and ||. I'll put it on the list but it will be a few weeks, at least before I get to it.

If you would like to tackle this I can guide you. The changes are extremely localized in just a few functions.

[brianguenter]

Was easier than I thought. There are two new functions conditional_and and conditional_or which provide the functionality of && and ||.

I overrode Base.isequal(x::Node,y::Node) to return x==y. Which I think is sensible but there could be subtle issues here I'm not seeing. Are you aware of any? If you want to check it out look at brianguenter/branch98.

I want to mull the isequal change over for a few days before I issue a patch release.

[moble]

Was easier than I thought.

Awesome! Thanks!

I overrode Base.isequal(x::Node,y::Node) to return x==y

I'm worried about this. Is it actually hard to just do the same thing for isequal as for ==? While I don't personally make a distinction, I support the distinction through my libraries. For example, I have a Quaternion type, where testing equality between two quaternions with == tests each pair of the four components with ==, whereas testing the two quaternions with isequal tests each pair of components with isequal. I don't have to make the distinction, but if my users do, it will work. Similarly, I think users of FD would expect the distinction to be preserved.

Just as a quick point of fact, here are what I think are the only three cases that are actually different, with the comments showing the result -> what the result would be with isequal.

-0.0 == 0.0  # true -> false
NaN == NaN  # false -> true
missing == missing  # missing -> true

So, for example, I can imagine someone might want to distinguish between -0.0 and 0.0 as the two different directions to approach the limit for some function that has discontinuous first derivative; their function would have to use isequal. If you change that to ==, it could lead to silent bugs that are hard to diagnose.

[brianguenter]

It would be easy to make it return an isequal node object instead of an == object. I could do that instead. Then the generated code would have the behavior you want.

[moble]

That sounds like a safer way to go.

[moble]

Anything I can do to help get this implemented?

[brianguenter]

I added isequal to the list of operators supported by FastDifferentiation. All the existing tests pass, but this is not surprising since none of them test isequal. If you could clone branch98 and run your tests on it to verify that it works as you expect that would be helpful. I'm in the middle of fixing another bug so pressed for time.

Make a PR with your new tests and I'll do a quick approval if everything looks okay.

One obvious problem with adding these conditionals is that if you do this:

julia> @variables x y
y

julia> f = isequal(x,y)
(x isequal y)

julia> g = make_function([f],[x,y])
RuntimeGeneratedFunction(#=in FastDifferentiation=#, #=using FastDifferentiation=#, :((input_variables,)->begin
          #= c:\Users\seatt\Source\FastDifferentiation.jl\src\CodeGeneration.jl:237 =#
          #= c:\Users\seatt\Source\FastDifferentiation.jl\src\CodeGeneration.jl:237 =# @inbounds begin
                  #= c:\Users\seatt\Source\FastDifferentiation.jl\src\CodeGeneration.jl:238 =#
                  begin
                      result_element_type = promote_type(Float64, eltype(input_variables))
                      result = Array{result_element_type}(undef, (1,))
                      var"##230" = isequal(input_variables[1], input_variables[2])
                      result[1] = var"##230"
                      return result
                  end
              end
      end))

julia> g([1.0,2.0])
1-element Vector{Float64}:
 0.0

the return type is Vector{Float64} when it should be Vector{Bool}. This will be true for all the new conditionals, not just isequal.

Could document it as a known issue that is not considered a bug, just something people have to workaround. Or could change the code generation to do a more sophisticated analysis to generate the correct return type, without also causing lots of run time overhead. This could get complicated. For example

julia> f = [isequal(x,y),x^2]
2-element Vector{FastDifferentiation.Node}:
   (x isequal y)
 (x ^ 2)

julia> g = make_function(f,[x,y])

julia> g([1.0,2.0])
2-element Vector{Float64}:
 0.0
 1.0

Should the return type of g be Vector{Any} or Vector{Union{Bool,Float64}? If the latter then how big a Union type should be allowed?

Or what about this case

julia> f = if_else(x<y,x==y,x^2)
(if_else  (x < y) (x == y) (x ^ 2))

julia> g = make_function([f],[x,y])
RuntimeGeneratedFunction(#=in FastDifferentiation=#, #=using FastDifferentiation=#, :((input_variables,)->begin
         @inbounds begin
                  begin
                      result_element_type = promote_type(Float64, eltype(input_variables))        
                      result = Array{result_element_type}(undef, (1,))
                      var"##234" = input_variables[1] < input_variables[2]
                      var"##233" = if var"##234" 
                              #= c:\Users\seatt\Source\FastDifferentiation.jl\src\CodeGeneration.jl:57 =#
                              begin
                                  var"##235" = input_variables[1] == input_variables[2]
                              end
                          else
                              #= c:\Users\seatt\Source\FastDifferentiation.jl\src\CodeGeneration.jl:59 =#
                              begin
                                  var"##236" = input_variables[1] ^ 2
                              end
                          end
                      result[1] = var"##233"     
                      return result
                  end
              end
      end))

julia> g([1.0,1.0])
1-element Vector{Float64}:
 1.0

julia> g([1.0,2.0])
1-element Vector{Float64}:
 0.0

How should this be handled? If x<y should it return a Bool vector and a Float64 vector otherwise? Or should we leave the return type as is and warn people in the documentation about what could happen if they return Bool values?

[moble]

I started putting together the tests, only to realize that I don't see how it's even possible for && and || to show up in a FastDifferentiation expression. That is, I don't see how this case could ever be triggered:

FastDifferentiation.jl/src/CodeGeneration.jl

Line 61 in 063d1a2

elseif value(node) === :(&&) || value(node) === :||

As soon as Julia sees Nodes in those expressions, it immediately errors out with

ERROR: TypeError: non-boolean (FastDifferentiation.Node) used in boolean context

I think this is because && and || are low-level operations, rather than infix operators that can be converted to function calls, and Julia just doesn't have the ability to deal with something other than a Bool with them. There's some discussion of this here. It looks like there are some knowledgeable people who want this to happen. I suppose wherever Julia deals with these expressions something could be put in so that instead of erroring, it does actually try to lower to something that could use multiple dispatch.

Short of support in the language, I guess the solution would be analogous to the ifelse situation, where people just don't use these short-circuiting operators if they want their code to be differentiable?

---

There's still value in this branch you made, so I'd be happy to remove the relevant lines and continue finishing it off for you. I think leaving the return type as Float64 is fine for now; since that conversion only happens for the output — rather than internal processing of the function — the conversion usually won't even happen, and when it does will still be correct or raise a TypeError.

[brianguenter]

The branch has two functions, conditional_and, and conditional_or that you call instead of using the && and || operators. A bit clunky but doesn't seem any way around it right now.

Here's an example:

julia> f = conditional_and(x > 0.0, y > sqrt(x))
(if_else  (x > 0.0) (y > sqrt(x)) false)

julia> g = make_function([f],[x,y])
RuntimeGeneratedFunction(#=in FastDifferentiation=#, #=using FastDifferentiation=#, :((input_variables,)->begin
          #= c:\Users\seatt\source\FastDifferentiation.jl\src\CodeGeneration.jl:237 =#   
          #= c:\Users\seatt\source\FastDifferentiation.jl\src\CodeGeneration.jl:237 =# @inbounds begin
                  #= c:\Users\seatt\source\FastDifferentiation.jl\src\CodeGeneration.jl:238 =#
                  begin
                      result_element_type = promote_type(Float64, eltype(input_variables))
                      result = Array{result_element_type}(undef, (1,))
                      var"##231" = input_variables[1] > 0.0
                      var"##230" = if var"##231"
                              #= c:\Users\seatt\source\FastDifferentiation.jl\src\CodeGeneration.jl:57 =#
                              begin
                                  var"##233" = sqrt(input_variables[1])
                                  var"##232" = input_variables[2] > var"##233"
                              end
                          else
                              #= c:\Users\seatt\source\FastDifferentiation.jl\src\CodeGeneration.jl:59 =#
                              begin
                                  var"##s#234" = false
                              end
                          end
                      result[1] = var"##230"
                      return result
                  end
              end
      end))

julia> g([-1.0,0.0])
1-element Vector{Float64}:
 0.0

julia> g([1.0,2.0])
1-element Vector{Float64}:
 1.0

[brianguenter]

@moble as soon as you get your tests finished I can review them and get this PR accepted.