Merge pull request #804 from JuliaSymbolics/as/no-hvncat

fix: fix more bugs

Author: AayushSabharwal

docs/pages.jl

@@ -1,9 +1,11 @@
 pages = Any["index.md",
-            "Manual" => Any["manual/representation.md",
+            "Manual" => Any["manual/variants.md",
                             "manual/rewrite.md",
                             "manual/interface.md",
                             "manual/codegen.md",
-                            "manual/variants.md"],
+                            "manual/caching.md",
+                            "manual/recursive_utils.md",
+                            ],
             "api.md",
             "upgrade.md"
             ]

docs/src/api.md

@@ -58,35 +58,3 @@ SymbolicUtils.simplify_fractions
 SymbolicUtils.quick_cancel
 SymbolicUtils.flatten_fractions
 ```
-
-## Code Generation
-
-### Core Functions
-```@docs
-SymbolicUtils.Code.toexpr
-SymbolicUtils.Code.cse
-```
-
-### Code Generation Types
-```@docs
-SymbolicUtils.Code.Assignment
-SymbolicUtils.Code.Let
-SymbolicUtils.Code.Func
-SymbolicUtils.Code.DestructuredArgs
-SymbolicUtils.Code.LiteralExpr
-SymbolicUtils.Code.ForLoop
-```
-
-### Array Operations
-```@docs
-SymbolicUtils.Code.SetArray
-SymbolicUtils.Code.MakeArray
-SymbolicUtils.Code.MakeSparseArray
-SymbolicUtils.Code.MakeTuple
-```
-
-### Parallelism
-```@docs
-SymbolicUtils.Code.SpawnFetch
-SymbolicUtils.Code.Multithreaded
-```

docs/src/manual/caching.md

@@ -0,0 +1,18 @@
+# Caching recursive functions
+
+Many functions benefit greatly from caching their results such that repeated calls with the same arguments
+can return cached values. As an example, `getindex` on symbolic arrays is a cached function since the operation can
+be expensive. SymbolicUtils.jl provides the `@cache` macro to allow easily caching such functions, with special
+benefits when the arguments are symbolic.
+
+```@docs
+SymbolicUtils.@cache
+SymbolicUtils.associated_cache
+SymbolicUtils.get_limit
+SymbolicUtils.set_limit
+SymbolicUtils.get_retain_fractions
+SymbolicUtils.set_retain_fractions
+SymbolicUtils.toggle_caching
+SymbolicUtils.is_caching_enabled
+SymbolicUtils.clear_cache
+```

docs/src/manual/codegen.md

@@ -35,4 +35,18 @@ SymbolicUtils.Code.MakeArray
 SymbolicUtils.Code.MakeSparseArray
 SymbolicUtils.Code.MakeTuple
 SymbolicUtils.Code.LiteralExpr
-```
+SymbolicUtils.Code.ForLoop
+```
+
+## Optimizations
+
+### Common Subexpression Elimination (CSE)
+
+SymbolicUtils can perform CSE on symbolic expressions, and codegen primitives composed of the above "Code Combinators".
+This ensures that common subexpressions in the expression are only computed once. Note that this assumes that all functions
+called within the expression are pure. SymbolicUtils can and will change the number and order of function calls.
+
+```@docs
+SymbolicUtils.Code.cse
+SymbolicUtils.Code.cse_inside_expr
+```

docs/src/manual/recursive_utils.md

@@ -0,0 +1,17 @@
+# Recursive utility functions
+
+SymbolicUtils.jl provides several utility functions to perform common operations that
+require recursing over the expression tree.
+
+```@docs
+SymbolicUtils.substitute
+SymbolicUtils.Substituter
+SymbolicUtils.default_substitute_filter
+SymbolicUtils.evaluate
+SymbolicUtils.query
+SymbolicUtils.search_variables
+SymbolicUtils.search_variables!
+SymbolicUtils.default_is_atomic
+SymbolicUtils.scalarize
+SymbolicUtils.scalarization_function
+```

docs/src/manual/representation.md

@@ -9,7 +9,7 @@ as `BasicSymbolic`. A `BasicSymbolic` is considered immutable. Mutating its fiel
 behavior.
 
 In SymbolicUtils v3, the type `T` in `BasicSymbolic{T}` was the type represented by the symbolic
-variable. In other words, `T` was the [`symtype`](@ref) of the variable.
+variable. In other words, `T` was the [`SymbolicUtils.symtype`](@ref) of the variable.
 
 In SymbolicUtils v4, the `symtype` is not stored in the type, and is instead a field of the
 struct. This allows for greatly increased type-stability. The type `T` in `BasicSymbolic{T}`
@@ -26,6 +26,7 @@ A given expression must be pure in its `vartype`. In other words, no operation s
 of different `vartype`s.
 
 !!! warning "A short note on (im-)mutability"
+  
   While `ismutabletype(BasicSymbolic)` returns `true`, symbolic types are IMMUTABLE.
   Any mutation is undefined behavior and can lead to very confusing and hard-to-debug issues.
   This includes internal mutation, such as mutating `AddMul.dict`. The arrays returned from
@@ -108,10 +109,10 @@ 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
-work on arrays of symbolics without excessive special-case handling and improved
-type-stability.
+(or array of array of ... symbolics), it will return a `Term` (see below) with
+[`SymbolicUtils.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.
 
 ```julia
 struct Sym
@@ -294,6 +295,7 @@ range of indices over which they should iterate, in case such a range is explici
 provided.
 
 !!! note
+  
   The common global index variable is printed as `_1`, `_2`, ... in arrayops. It is not
   a valid symbolic variable outside of an `ArrayOp`'s `expr`.
 
@@ -336,8 +338,8 @@ to Base-like behavior:
   doing so requires the function(s) passed to `map` and `mapreduce` instead of their types
   or shapes.
 - Since `ndims` information is not present in the type, `eachindex`, `iterate`, `size`,
-  `axes`, `ndims`, `collect` are type-unstable. [`safe_eachindex`](@ref) is useful as
-  a type-stable iteration alternative.
+  `axes`, `ndims`, `collect` are type-unstable. [`SymbolicUtils.stable_eachindex`](@ref) is
+  useful as a type-stable iteration alternative.
 - `ifelse` requires that both the true and false cases have identical shape.
 - Symbolic arrays _only_ support cartesian indexing. For example, given `@syms x[1:3, 1:3]`
   accessing `x[4]` is invalid and `x[1, 2]` should be used. Valid indices are
@@ -385,7 +387,7 @@ words,
 Here, `f1(x)` is considered a symbolic function `f1` called with the argument `x` and
 `f2(x)` is considered a dependent variable that depends on `x`. The utilities
 [`SymbolicUtils.is_function_symbolic`](@ref), [`SymbolicUtils.is_function_symtype`](@ref),
-[`symbolicUtils.is_called_function_symbolic`](@ref) can be used to differentiate between
+[`SymbolicUtils.is_called_function_symbolic`](@ref) can be used to differentiate between
 these cases.
 
 ## API
@@ -409,6 +411,7 @@ unwrap_const
 ### Inner constructors
 
 ```@docs
+SymbolicUtils.array_literal
 SymbolicUtils.BSImpl.Const
 SymbolicUtils.BSImpl.Sym
 SymbolicUtils.BSImpl.Term
@@ -457,9 +460,9 @@ SymbolicUtils.promote_shape
 ### Symbolic array utilities
 
 ```@docs
-SymbolicUtils.safe_eachindex
-SymbolicUtils.SafeIndices
-SymbolicUtils.SafeIndex
+SymbolicUtils.stable_eachindex
+SymbolicUtils.StableIndices
+SymbolicUtils.StableIndex
 BS
 ```
 

docs/src/manual/variants.md

@@ -18,8 +18,7 @@ using SymbolicIndexingInterface
 import Base: +, -, *, /, //, \, ^, ImmutableDict
 using ConstructionBase
 using TermInterface
-import TermInterface: iscall, isexpr, children,
-                      operation, arguments, metadata, maketerm, sorted_arguments
+import TermInterface: iscall, operation, arguments, metadata, maketerm, sorted_arguments
 # For ReverseDiffExt
 import ArrayInterface
 import ExproniconLite as EL

src/SymbolicUtils.jl

@@ -869,10 +869,9 @@ end
 """
     $(TYPEDSIGNATURES)
 
-Return `true` if CSE should descend inside `sym`, which has operation `f` and
-arguments `args...`.
+Return `true` if CSE should descend inside `sym`, which has operation `f`.
 """
-function cse_inside_expr(sym, f, args...)
+function cse_inside_expr(sym, f)
     return true
 end
 
@@ -1009,7 +1008,7 @@ function cse!(expr::BasicSymbolic{T}, state::CSEState) where {T}
                 if op isa BasicSymbolic{T}
                     SymbolicUtils.is_function_symbolic(op) || return expr
                 end
-                cse_inside_expr(expr, op, args...) || return expr
+                cse_inside_expr(expr, op) || return expr
                 args = map(Base.Fix2(cse!, state), args)
                 # use `term` instead of `maketerm` because we only care about the operation being performed
                 # and not the representation. This avoids issues with `newsym` symbols not having sizes, etc.

src/code.jl

@@ -33,7 +33,7 @@ the expression.
 This function is used internally for printing via AbstractTrees.
 """
 function AbstractTrees.children(x::BasicSymbolic)
-    iscall(x) ? sorted_arguments(x) : isexpr(x) ? sorted_children(x) : ()
+    iscall(x) ? sorted_arguments(x) : ()
 end
 
 """