refactor: use new array_literal instead of hvncat for arrays of symbolics

Author: AayushSabharwal

docs/src/manual/variants.md

@@ -108,8 +108,8 @@ variant. This variant can be constructed using `Const{T}(val)` or `BSImpl.Const{
 where `T` is the appropriate `vartype`.
 
 The `Const` constructors have an additional special behavior. If given an array of symbolics
-(or array of array of ... symbolics), it will return a `Term` (see below) with `hvncat` as
-the operation. This allows standard symbolic operations (such as [`substitute`](@ref)) to
+(or array of array of ... symbolics), it will return a `Term` (see below) with [`array_literal`](@ref)
+as the operation. This allows standard symbolic operations (such as [`substitute`](@ref)) to
 work on arrays of symbolics without excessive special-case handling and improved
 type-stability.
 
@@ -409,6 +409,7 @@ unwrap_const
 ### Inner constructors
 
 ```@docs
+SymbolicUtils.array_literal
 SymbolicUtils.BSImpl.Const
 SymbolicUtils.BSImpl.Sym
 SymbolicUtils.BSImpl.Term

src/methods.jl

@@ -238,7 +238,7 @@ end
 _sequential_promote(T::TypeT) = T
 
 
-function promote_symtype(::typeof(hvncat), Tp::TypeT, Ts::TypeT...)
+function promote_symtype(::typeof(array_literal), Tp::TypeT, Ts::TypeT...)
     @assert Tp <: Tuple
     return Array{_sequential_promote(Ts...), length(Tp.parameters)}
 end

src/types.jl

@@ -1484,6 +1484,8 @@ function _is_tuple_of_symbolics(O::Tuple)
 end
 _is_tuple_of_symbolics(O) = false
 
+array_literal(sz::NTuple{N, Int}, args...) where {N} = reshape(Base.vect(args...), sz)
+
 """
     BSImpl.Const{T}(val) where {T}
 
@@ -1500,7 +1502,7 @@ arrays/tuples of symbolics to symbolic expressions.
 This is the low-level constructor for constant expressions. It handles several special cases:
 1. If `val` is already a `BasicSymbolic{T}`, returns it unchanged
 2. If `val` is a `BasicSymbolic` of a different variant type, throws an error
-3. If `val` is an array containing symbolic elements, creates a `Term` with `hvncat` operation
+3. If `val` is an array containing symbolic elements, creates a `Term` with [`array_literal`](@ref) operation
 4. If `val` is a tuple containing symbolic elements, creates a `Term` with `tuple` operation
 5. Otherwise, creates a `Const` variant wrapping the value
 
@@ -1520,15 +1522,15 @@ The `unsafe` flag skips hash consing for performance in internal operations.
     elseif val isa BasicSymbolic{TreeReal}
         error("Cannot construct `BasicSymbolic{$T}` from `BasicSymbolic{TreeReal}`.")
     elseif val isa AbstractArray && _is_array_of_symbolics(val)
-        args = ArgsT{T}((BSImpl.Const{T}(size(val); unsafe), BSImpl.Const{T}(false; unsafe)))
-        sizehint!(args, length(val) + 2)
+        args = ArgsT{T}((BSImpl.Const{T}(size(val); unsafe),))
+        sizehint!(args, length(val) + 1)
         type = Union{}
         for v in val
             push!(args, BSImpl.Const{T}(v))
             type = promote_type(type, symtype(v))
         end
         shape = ShapeVecT(axes(val))
-        return BSImpl.Term{T}(hvncat, args; type = Array{type, ndims(val)}, shape, unsafe)
+        return BSImpl.Term{T}(array_literal, args; type = Array{type, ndims(val)}, shape, unsafe)
     elseif val isa Tuple && _is_tuple_of_symbolics(val)
         args = ArgsT{T}()
         sizehint!(args, length(val))
@@ -2470,7 +2472,7 @@ function TermInterface.maketerm(::Type{BasicSymbolic{T}}, f, args, metadata; @no
             @set! res.metadata = metadata
         end
         return res::BasicSymbolic{T}
-    elseif f === hvncat
+    elseif f === array_literal
         sh = ShapeVecT()
         for dim in unwrap_const(args[1])
             push!(sh, 1:dim)
@@ -3998,8 +4000,8 @@ function __stable_getindex(arr::BasicSymbolic{T}, sidxs::StableIndex) where {T}
     sh::ShapeVecT = shape(arr)
     @match arr begin
         BSImpl.Const(; val) => return Const{T}(scalar_index(val, as_linear_idx(sh, sidxs)))
-        BSImpl.Term(; f, args) && if f === hvncat end => begin
-            return args[2 + as_linear_idx(sh, sidxs)]
+        BSImpl.Term(; f, args) && if f === array_literal  end => begin
+            return args[1 + as_linear_idx(sh, sidxs)]
         end
         BSImpl.Term(; f, args) && if f isa TypeT && f <: CartesianIndex end => begin
             return args[as_linear_idx(sh, sidxs)]
@@ -4069,8 +4071,8 @@ end
 Base.@propagate_inbounds function _getindex(::Type{T}, arr::BasicSymbolic{T}, idxs::Union{BasicSymbolic{T}, Int, AbstractRange{Int}, Colon}...) where {T}
     @match arr begin
         BSImpl.Const(; val) && if all(x -> !(x isa BasicSymbolic{T}) || isconst(x), idxs) end => return Const{T}(val[unwrap_const.(idxs)...])
-        BSImpl.Term(; f) && if f === hvncat && all(x -> !(x isa BasicSymbolic{T}) || isconst(x), idxs) end => begin
-            return Const{T}(reshape(@view(arguments(arr)[3:end]), Tuple(size(arr)))[unwrap_const.(idxs)...])
+        BSImpl.Term(; f) && if f === array_literal && all(x -> !(x isa BasicSymbolic{T}) || isconst(x), idxs) end => begin
+            return Const{T}(reshape(@view(arguments(arr)[2:end]), Tuple(size(arr)))[unwrap_const.(idxs)...])
         end
         BSImpl.Term(; f, args) && if f isa TypeT && f <: CartesianIndex end => return args[idxs...]
         BSImpl.Term(; f, args) && if f isa Operator && length(args) == 1 end => begin

test/basics.jl

@@ -217,13 +217,13 @@ end
     var = Const{SymReal}(symvec)
     @test var isa BasicSymbolic{SymReal}
     @test isterm(var)
-    @test isequal(arguments(var), Const{SymReal}.([(2,), false, h, x]))
+    @test isequal(arguments(var), Const{SymReal}.([(2,), h, x]))
     @test symtype(var) == Vector{Number}
     @test shape(var) == ShapeVecT((1:2,))
     var = Const{SymReal}(symmat)
     @test var isa BasicSymbolic{SymReal}
     @test isterm(var)
-    @test isequal(arguments(var), Const{SymReal}.([(2, 2), false, h, y, x, z]))
+    @test isequal(arguments(var), Const{SymReal}.([(2, 2), h, y, x, z]))
     @test symtype(var) == Matrix{Number}
     @test shape(var) == ShapeVecT((1:2, 1:2))
     csymvec = Const{SymReal}(symvec)

test/cse.jl

@@ -61,9 +61,9 @@ end
     end
     ex = term(foo, [a^2 + b^2, b^2 + c], (a^2 + b^2, b^2 + c), c; type = Real)
     sorted_nodes = topological_sort(ex)
-    @test length(sorted_nodes) == 10
-    @test operation(sorted_nodes[8].rhs) === hvncat
-    @test operation(sorted_nodes[9].rhs) === tuple
+    @test length(sorted_nodes) == 9
+    @test operation(sorted_nodes[7].rhs) === SymbolicUtils.array_literal
+    @test operation(sorted_nodes[8].rhs) === tuple
     expr = quote
         a = 1
         b = 2

test/methods.jl

@@ -1,5 +1,5 @@
 using SymbolicUtils
-using SymbolicUtils: Sym, Term, symtype, BasicSymbolic, Const, ArgsT, promote_symtype, promote_shape, ShapeVecT, Unknown
+using SymbolicUtils: Sym, Term, symtype, BasicSymbolic, Const, ArgsT, promote_symtype, promote_shape, ShapeVecT, Unknown, array_literal
 using Test
 import NaNMath
 import LinearAlgebra
@@ -72,9 +72,9 @@ end
     @test promote_shape(identity, ShapeVecT()) == ShapeVecT()
     @test promote_shape(identity, ShapeVecT((1:2, 1:3))) == ShapeVecT((1:2, 1:3))
 end
-@testset "promote_symtype for hvncat" begin
-    @test promote_symtype(hvncat, NTuple{2, Int}, Int, Float64, Int32) == Array{Float64, 2}
-    @test promote_symtype(hvncat, NTuple{3, Int}, Int, Int, Int) == Array{Int, 3}
+@testset "promote_symtype for `array_literal`" begin
+    @test promote_symtype(array_literal, NTuple{2, Int}, Int, Float64, Int32) == Array{Float64, 2}
+    @test promote_symtype(array_literal, NTuple{3, Int}, Int, Int, Int) == Array{Int, 3}
 end
 
 @testset "promote_symtype for rem2pi" begin

test/misc.jl

@@ -227,11 +227,10 @@ end
     # Test array of symbolics
     arr = [x, y, x+y]
     const_arr = SymbolicUtils.Const{SymbolicUtils.SymReal}(arr)
-    @test operation(const_arr) === hvncat
+    @test operation(const_arr) === SymbolicUtils.array_literal
     args = arguments(const_arr)
     @test unwrap_const(args[1]) == (3,)
-    @test unwrap_const(args[2]) == false
-    @test length(args) == 5
+    @test length(args) == 4
 
     # Test tuple of symbolics
     tup = (x, y)