Merge pull request #1770 from JuliaRobotics/master

v0.34.1-rc1

NEWS.md

@@ -1,4 +1,4 @@
-# Additional NEWS on IncrementalInference.jl Releases
+# NEWS on IncrementalInference.jl Releases
 
 Currently general maintenance and bug fix changes are mostly tracked via Github Integrations.  E.g. see Milestones along with Label filters to quickly find specific issues.
 - https://github.com/JuliaRobotics/IncrementalInference.jl/milestones?state=closed
@@ -13,8 +13,12 @@ The list below highlights breaking changes according to normal semver workflow -
 
 # Changes in v0.34
 
-- Start transition to Manopt.jl via Riemannian Levenberg Marquart.
+- Start transition to Manopt.jl via Riemannian Levenberg-Marquart.
 - Deprecate `AbstractRelativeRoots`.
+- Standardization improvements surrounding weakdeps code extensions. 
+- Code quality improvements along wiht refactoring and reorganizing of file names and locations.
+- Restoring `DERelative` factors, although further fixes necessary beyond anticipated patch release v0.34.1.
+- Switching to weakdep AMD.jl for `ccolmod` dependency, part of Julia 1.10 upgrade.  Dropping `SuiteSparse_long` dependency.  Further fixes necessary to restore full user constrained tree variable order functionality.
 
 # Changes in v0.33
 

Project.toml

@@ -2,7 +2,7 @@ name = "IncrementalInference"
 uuid = "904591bb-b899-562f-9e6f-b8df64c7d480"
 keywords = ["MM-iSAMv2", "Bayes tree", "junction tree", "Bayes network", "variable elimination", "graphical models", "SLAM", "inference", "sum-product", "belief-propagation"]
 desc = "Implements the Multimodal-iSAMv2 algorithm."
-version = "0.34.0"
+version = "0.34.1"
 
 [deps]
 ApproxManifoldProducts = "9bbbb610-88a1-53cd-9763-118ce10c1f89"
@@ -49,21 +49,23 @@ TimeZones = "f269a46b-ccf7-5d73-abea-4c690281aa53"
 UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4"
 
 [weakdeps]
+AMD = "14f7f29c-3bd6-536c-9a0b-7339e30b5a3e"
 DifferentialEquations = "0c46a032-eb83-5123-abaf-570d42b7fbaa"
 Flux = "587475ba-b771-5e3f-ad9e-33799f191a9c"
 Gadfly = "c91e804a-d5a3-530f-b6f0-dfbca275c004"
 InteractiveUtils = "b77e0a4c-d291-57a0-90e8-8db25a27a240"
 Interpolations = "a98d9a8b-a2ab-59e6-89dd-64a1c18fca59"
 
 [extensions]
+IncrInfrApproxMinDegreeExt = "AMD"
 IncrInfrDiffEqFactorExt = "DifferentialEquations"
 IncrInfrFluxFactorsExt = "Flux"
 IncrInfrGadflyExt = "Gadfly"
 IncrInfrInteractiveUtilsExt = "InteractiveUtils"
 IncrInfrInterpolationsExt = "Interpolations"
 
 [compat]
-ApproxManifoldProducts = "0.7"
+ApproxManifoldProducts = "0.7, 0.8"
 BSON = "0.2, 0.3"
 BlockArrays = "0.16"
 Combinatorics = "1.0"
@@ -98,6 +100,7 @@ TimeZones = "1.3.1"
 julia = "1.9"
 
 [extras]
+AMD = "14f7f29c-3bd6-536c-9a0b-7339e30b5a3e"
 Graphs = "86223c79-3864-5bf0-83f7-82e725a168b6"
 LineSearches = "d3d80556-e9d4-5f37-9878-2ab0fcc64255"
 Manopt = "0fc0a36d-df90-57f3-8f93-d78a9fc72bb5"
@@ -107,4 +110,4 @@ Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 Zygote = "e88e6eb3-aa80-5325-afca-941959d7151f"
 
 [targets]
-test = ["DifferentialEquations", "Flux", "Graphs", "Manopt", "InteractiveUtils", "Interpolations", "LineSearches", "Pkg", "Rotations", "Test", "Zygote"]
+test = ["AMD", "DifferentialEquations", "Flux", "Graphs", "Manopt", "InteractiveUtils", "Interpolations", "LineSearches", "Pkg", "Rotations", "Test", "Zygote"]

ext/IncrInfrApproxMinDegreeExt.jl

@@ -0,0 +1,100 @@
+module IncrInfrApproxMinDegreeExt
+
+using AMD
+import IncrementalInference: _ccolamd, _ccolamd!
+
+# elseif ordering == :ccolamd
+#   cons = zeros(SuiteSparse_long, length(adjMat.colptr) - 1)
+#   cons[findall(x -> x in constraints, permuteds)] .= 1
+#   p = Ccolamd.ccolamd(adjMat, cons)
+#   @warn "Ccolamd is experimental in IIF at this point in time."
+
+const KNOBS = 20
+const STATS = 20
+
+
+
+function _ccolamd!(
+  n_row, #SuiteSparse_long,
+  A::AbstractVector{T}, # SuiteSparse_long},
+  p::AbstractVector, # SuiteSparse_long},
+  knobs::Union{Ptr{Nothing}, Vector{Float64}},
+  stats::AbstractVector, #{SuiteSparse_long},
+  cmember::Union{Ptr{Nothing}, <:AbstractVector}, #{SuiteSparse_long}},
+) where T
+  n_col = length(p) - 1
+
+  if length(stats) != STATS
+    error("stats must hcae length $STATS")
+  end
+  if isa(cmember, Vector) && length(cmember) != n_col
+    error("cmember must have length $n_col")
+  end
+
+  Alen = AMD.ccolamd_l_recommended(length(A), n_row, n_col)
+  resize!(A, Alen)
+
+  for i in eachindex(A)
+    A[i] -= 1
+  end
+  for i in eachindex(p)
+    p[i] -= 1
+  end
+  err = AMD.ccolamd_l( # ccolamd_l
+    n_row,
+    n_col,
+    Alen,
+    A,
+    p,
+    knobs,
+    stats,
+    cmember
+  )
+
+  if err == 0
+    AMD.ccolamd_l_report(stats)
+    error("call to ccolamd return with error code $(stats[4])")
+  end
+
+  for i in eachindex(p)
+    p[i] += 1
+  end
+
+  pop!(p) # remove last zero from pivoting vector
+  return p
+end
+
+function _ccolamd!(
+  n_row,
+  A::AbstractVector{T1}, #SuiteSparse_long},
+  p::AbstractVector{<:Real}, # {SuiteSparse_long},
+  cmember::Union{Ptr{Nothing}, <:AbstractVector{T}}, # SuiteSparse_long
+) where {T1<:Real, T}
+  n_col = length(p) - 1
+
+  if length(cmember) != n_col
+    error("cmember must have length $n_col")
+  end
+
+  Alen = AMD.ccolamd_l_recommended(length(A), n_row, n_col)
+  resize!(A, Alen)
+  stats = zeros(T1, STATS)
+  return _ccolamd!(n_row, A, p, C_NULL, stats, cmember)
+end
+
+# function _ccolamd!(
+#   n_row,
+#   A::AbstractVector{T}, # ::Vector{SuiteSparse_long},
+#   p::AbstractVector, # ::Vector{SuiteSparse_long},
+#   constraints = zeros(T,length(p) - 1), # SuiteSparse_long, 
+# ) where T
+#   n_col = length(p) - 1
+#   return _ccolamd!(n_row, A, p, constraints)
+# end
+
+_ccolamd(n_row,A,p,constraints) = _ccolamd!(n_row, copy(A), copy(p), constraints)
+_ccolamd(biadjMat, constraints) = _ccolamd(size(biadjMat, 1), biadjMat.rowval, biadjMat.colptr, constraints)
+
+
+
+end

ext/IncrInfrDiffEqFactorExt.jl

@@ -9,6 +9,7 @@ using Dates
 
 using IncrementalInference
 import IncrementalInference: getSample, getManifold, DERelative
+import IncrementalInference: sampleFactor
 
 using DocStringExtensions
 
@@ -94,12 +95,12 @@ end
 #
 #
 
-# Xtra splat are variable points (X3::Matrix, X4::Matrix,...)
+# n-ary factor: Xtra splat are variable points (X3::Matrix, X4::Matrix,...)
 function _solveFactorODE!(measArr, prob, u0pts, Xtra...)
-  # should more variables be included in calculation
+  # happens when more variables (n-ary) must be included in DE solve
   for (xid, xtra) in enumerate(Xtra)
     # update the data register before ODE solver calls the function
-    prob.p[xid + 1][:] = Xtra[xid][:]
+    prob.p[xid + 1][:] = xtra[:]
   end
 
   # set the initial condition
@@ -111,47 +112,47 @@ function _solveFactorODE!(measArr, prob, u0pts, Xtra...)
   return sol
 end
 
-getSample(cf::CalcFactor{<:DERelative}) = error("getSample(::CalcFactor{<:DERelative}) must still be implemented in new IIF design")
+# # # output for AbstractRelative is tangents (but currently we working in coordinates for integration with DiffEqs)
+# # # FIXME, how to consolidate DERelative with parametric solve which currently only goes through getMeasurementParametric
+# function getSample(cf::CalcFactor{<:DERelative})
+#   #
+#   oder = cf.factor
+
+#   # how many trajectories to propagate?
+#   # @show getLabel(cf.fullvariables[2]), getDimension(cf.fullvariables[2])
+#   meas = zeros(getDimension(cf.fullvariables[2])) 
+
+#   # pick forward or backward direction
+#   # set boundary condition
+#   u0pts = if cf.solvefor == 1
+#     # backward direction
+#     prob = oder.backwardProblem
+#     addOp, diffOp, _, _ = AMP.buildHybridManifoldCallbacks(
+#       convert(Tuple, getManifold(getVariableType(cf.fullvariables[1]))),
+#     )
+#     # FIXME use ccw.varValsAll containter?
+#     (getBelief(cf.fullvariables[2]) |> getPoints)[cf._sampleIdx]
+#   else
+#     # forward backward
+#     prob = oder.forwardProblem
+#     # buffer manifold operations for use during factor evaluation
+#     addOp, diffOp, _, _ = AMP.buildHybridManifoldCallbacks(
+#       convert(Tuple, getManifold(getVariableType(cf.fullvariables[2]))),
+#     )
+#     # FIXME use ccw.varValsAll containter?
+#     (getBelief(cf.fullvariables[1]) |> getPoints)[cf._sampleIdx]
+#   end
+
+#   # solve likely elements
+#   # TODO, does this respect hyporecipe ???
+#   # TBD check if cf._legacyParams == ccw.varValsAll???
+#   idxArr = (k -> cf._legacyParams[k][cf._sampleIdx]).(1:length(cf._legacyParams))
+#   _solveFactorODE!(meas, prob, u0pts, _maketuplebeyond2args(idxArr...)...)
+#   # _solveFactorODE!(meas, prob, u0pts, i, _maketuplebeyond2args(cf._legacyParams...)...)
+
+#   return meas, diffOp
+# end
 
-# FIXME see #1025, `multihypo=` will not work properly yet
-function sampleFactor(cf::CalcFactor{<:DERelative}, N::Int = 1)
-  #
-  oder = cf.factor
-
-  # how many trajectories to propagate?
-  # @show getLabel(cf.fullvariables[2]), getDimension(cf.fullvariables[2])
-  meas = [zeros(getDimension(cf.fullvariables[2])) for _ = 1:N]
-
-  # pick forward or backward direction
-  # set boundary condition
-  u0pts = if cf.solvefor == 1
-    # backward direction
-    prob = oder.backwardProblem
-    addOp, diffOp, _, _ = AMP.buildHybridManifoldCallbacks(
-      convert(Tuple, getManifold(getVariableType(cf.fullvariables[1]))),
-    )
-    getBelief(cf.fullvariables[2]) |> getPoints
-  else
-    # forward backward
-    prob = oder.forwardProblem
-    # buffer manifold operations for use during factor evaluation
-    addOp, diffOp, _, _ = AMP.buildHybridManifoldCallbacks(
-      convert(Tuple, getManifold(getVariableType(cf.fullvariables[2]))),
-    )
-    getBelief(cf.fullvariables[1]) |> getPoints
-  end
-
-  # solve likely elements
-  for i = 1:N
-    # TODO, does this respect hyporecipe ???
-    idxArr = (k -> cf._legacyParams[k][i]).(1:length(cf._legacyParams))
-    _solveFactorODE!(meas[i], prob, u0pts[i], _maketuplebeyond2args(idxArr...)...)
-    # _solveFactorODE!(meas, prob, u0pts, i, _maketuplebeyond2args(cf._legacyParams...)...)
-  end
-
-  return map(x -> (x, diffOp), meas)
-end
-# getDimension(oderel.domain)
 
 # NOTE see #1025, CalcFactor should fix `multihypo=` in `cf.__` fields; OBSOLETE
 function (cf::CalcFactor{<:DERelative})(measurement, X...)
@@ -197,6 +198,56 @@ function (cf::CalcFactor{<:DERelative})(measurement, X...)
   return res
 end
 
+
+
+
+## =========================================================================
+## MAYBE legacy
+
+# FIXME see #1025, `multihypo=` will not work properly yet
+function IncrementalInference.sampleFactor(cf::CalcFactor{<:DERelative}, N::Int = 1)
+  #
+  oder = cf.factor
+
+  # how many trajectories to propagate?
+  # @show getLabel(cf.fullvariables[2]), getDimension(cf.fullvariables[2])
+  meas = [zeros(getDimension(cf.fullvariables[2])) for _ = 1:N]
+
+  # pick forward or backward direction
+  # set boundary condition
+  u0pts = if cf.solvefor == 1
+    # backward direction
+    prob = oder.backwardProblem
+    addOp, diffOp, _, _ = AMP.buildHybridManifoldCallbacks(
+      convert(Tuple, getManifold(getVariableType(cf.fullvariables[1]))),
+    )
+    getBelief(cf.fullvariables[2]) |> getPoints
+  else
+    # forward backward
+    prob = oder.forwardProblem
+    # buffer manifold operations for use during factor evaluation
+    addOp, diffOp, _, _ = AMP.buildHybridManifoldCallbacks(
+      convert(Tuple, getManifold(getVariableType(cf.fullvariables[2]))),
+    )
+    getBelief(cf.fullvariables[1]) |> getPoints
+  end
+
+  # solve likely elements
+  for i = 1:N
+    # TODO, does this respect hyporecipe ???
+    idxArr = (k -> cf._legacyParams[k][i]).(1:length(cf._legacyParams))
+    _solveFactorODE!(meas[i], prob, u0pts[i], _maketuplebeyond2args(idxArr...)...)
+    # _solveFactorODE!(meas, prob, u0pts, i, _maketuplebeyond2args(cf._legacyParams...)...)
+  end
+
+  return map(x -> (x, diffOp), meas)
+end
+# getDimension(oderel.domain)
+
+
+
+
+
 ## the function
 # ode.problem.f.f
 

ext/WeakDepsPrototypes.jl

@@ -1,4 +1,8 @@
 
+# AMD.jl
+function _ccolamd! end
+function _ccolamd end
+
 # Flux.jl
 function MixtureFluxModels end
 

src/IncrementalInference.jl

@@ -56,10 +56,11 @@ using MetaGraphs
 using Logging
 using PrecompileTools
 
-# bringing in BSD 3-clause ccolamd
-include("services/ccolamd.jl")
-using SuiteSparse.CHOLMOD: SuiteSparse_long # For CCOLAMD constraints.
-using .Ccolamd
+# JL 1.10 transition to IncrInfrApproxMinDegreeExt instead
+# # bringing in BSD 3-clause ccolamd
+# include("services/ccolamd.jl")
+# using SuiteSparse.CHOLMOD: SuiteSparse_long # For CCOLAMD constraints.
+# using .Ccolamd
 
 # likely overloads or not exported by the upstream packages
 import Base: convert, ==, getproperty
@@ -129,7 +130,8 @@ include("entities/FactorOperationalMemory.jl")
 include("Factors/GenericMarginal.jl")
 # Special belief types for sampling as a distribution
 include("entities/AliasScalarSampling.jl")
-include("entities/OptionalDensities.jl") # used in BeliefTypes.jl::SamplableBeliefs
+include("entities/ExtDensities.jl") # used in BeliefTypes.jl::SamplableBeliefs
+include("entities/ExtFactors.jl")
 include("entities/BeliefTypes.jl")
 
 include("services/HypoRecipe.jl")
@@ -233,7 +235,7 @@ include("services/SolverAPI.jl")
 # Symbolic tree analysis files.
 include("services/AnalysisTools.jl")
 
-# optional densities on weakdeps
+# extension densities on weakdeps
 include("Serialization/entities/SerializingOptionalDensities.jl")
 include("Serialization/services/SerializingOptionalDensities.jl")