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

Inconsistent output order of build_function #1382

Closed
michakraus opened this issue Dec 12, 2024 · 8 comments
Closed

Inconsistent output order of build_function #1382

michakraus opened this issue Dec 12, 2024 · 8 comments
Assignees

Comments

@michakraus
Copy link

michakraus commented Dec 12, 2024

Depending on what I feed into build_function, the order of the output varies, i.e., out-of-place first or in-place first. With Symbolics v6.22.0, the following MWE

using Symbolics

@variables a[1:2]
@variables b[1:2]

f = a .- b
g = [a[i] - b[i] for i in eachindex(a,b)]

println("typeof(f) = $(typeof(f))")
println("typeof(g) = $(typeof(g))")

F = build_function(f, a, b; nanmath = false)
G = build_function(g, a, b; nanmath = false)

println("\nThis is F:")
println(F)

println("\nThis is G:")
println(G)

produces

typeof(f) = Symbolics.Arr{Num, 1}
typeof(g) = Vector{Num}

This is F:
(:(function (ˍ₋out, a, b)
      #= /Users/mkraus/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:385 =#
      #= /Users/mkraus/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:386 =#
      #= /Users/mkraus/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:387 =#
      begin
          #= /Users/mkraus/.julia/packages/Symbolics/YbNrd/src/arrays.jl:1124 =#
          for (i, i′) = zip(1:2, reset_to_one(1:2))
              #= /Users/mkraus/.julia/packages/Symbolics/YbNrd/src/arrays.jl:1112 =#
              ˍ₋out[i′] = (+)(ˍ₋out[i′], (-)((getindex)(a, i), (getindex)(b, i)))
              #= /Users/mkraus/.julia/packages/Symbolics/YbNrd/src/arrays.jl:1113 =#
          end
      end
  end), :(function (a, b)
      #= /Users/mkraus/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:385 =#
      #= /Users/mkraus/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:386 =#
      #= /Users/mkraus/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:387 =#
      (broadcast)(-, a, b)
  end))

This is G:
(:(function (a, b)
      #= /Users/mkraus/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:385 =#
      #= /Users/mkraus/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:386 =#
      #= /Users/mkraus/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:387 =#
      begin
          #= /Users/mkraus/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:480 =#
          (SymbolicUtils.Code.create_array)(Array, nothing, Val{1}(), Val{(2,)}(), (+)((getindex)(a, 1), (*)(-1, (getindex)(b, 1))), (+)((getindex)(a, 2), (*)(-1, (getindex)(b, 2))))
      end
  end), :(function (ˍ₋out, a, b)
      #= /Users/mkraus/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:385 =#
      #= /Users/mkraus/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:386 =#
      #= /Users/mkraus/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:387 =#
      begin
          #= /Users/mkraus/.julia/packages/Symbolics/YbNrd/src/build_function.jl:546 =#
          #= /Users/mkraus/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:434 =# @inbounds begin
                  #= /Users/mkraus/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:430 =#
                  ˍ₋out[1] = (+)((getindex)(a, 1), (*)(-1, (getindex)(b, 1)))
                  ˍ₋out[2] = (+)((getindex)(a, 2), (*)(-1, (getindex)(b, 2)))
                  #= /Users/mkraus/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:432 =#
                  nothing
              end
      end
  end))

Is this intended behaviour?

@ChrisRackauckas
Copy link
Member

This is not intended. In-place is always supposed to be first.

@michakraus
Copy link
Author

Are there any plans to fix this?

@AayushSabharwal
Copy link
Member

Right now everything returns (oop, iip)

@ChrisRackauckas
Copy link
Member

That's what I expected, but even in this MWE?

@AayushSabharwal
Copy link
Member

Yep, I had addressed this in one of the PRs that fixed build_function

@AayushSabharwal
Copy link
Member

@michakraus
Copy link
Author

Indeed, just tested with Symbolics v6.29.0 and the outputs are now consistent. Thanks for fixing this!

@ChrisRackauckas
Copy link
Member

oop is also always first, https://github.com/SciML/ModelingToolkit.jl/blame/ddcf59e7c50b091a8bbd4dd1aa61bdb40598a654/src/systems/diffeqs/abstractodesystem.jl#L333 is a 3 year old snippet

That's what I thought. I'm curious what happened to make this MWE, but whatever it was, it seems like it's gone.

@bowenszhu bowenszhu marked this as a duplicate of #712 Feb 11, 2025
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

3 participants