Skip to content

Commit 1e4b860

Browse files
committed
fixes, more on AbsManiMin
1 parent 5f2c0d8 commit 1e4b860

10 files changed

+62
-64
lines changed

NEWS.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,14 @@ Alternatively, either use the Github Blame, or the Github `/compare/v0.18.0...v0
1111

1212
The list below highlights breaking changes according to normal semver workflow -- i.e. breaking changes go through at least one deprecatation (via warnings) on the dominant number in the version number. E.g. v0.18 -> v0.19 (warnings) -> v0.20 (breaking). Note that ongoing efforts are made to properly deprecate old code/APIs
1313

14+
# Changes in v0.34
15+
16+
- Start transition to Manopt.jl via Riemannian Levenberg Marquart.
17+
- Deprecate `AbstractRelativeRoots`.
18+
19+
# Changes in v0.33
20+
21+
- Upgrades for DFG using StructTypes.jl (serialization).
1422
# Changes in v0.32
1523

1624
- Major internal refactoring of `CommonConvWrapper` to avoid abstract field types, and better standardization; towards cleanup of internal multihypo handling and naming conventions.

src/Deprecated.jl

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,29 @@ function solveGraphParametric2(
119119
return d, result, flatvar.idx, Σ
120120
end
121121

122+
123+
##==============================================================================
124+
## Deprecate code below before v0.35
125+
##==============================================================================
126+
127+
128+
129+
function _solveLambdaNumeric(
130+
fcttype::Union{F, <:Mixture{N_, F, S, T}},
131+
objResX::Function,
132+
residual::AbstractVector{<:Real},
133+
u0::AbstractVector{<:Real},
134+
islen1::Bool = false,
135+
) where {N_, F <: AbstractRelativeRoots, S, T}
136+
#
137+
138+
#
139+
r = NLsolve.nlsolve((res, x) -> res .= objResX(x), u0; inplace = true) #, ftol=1e-14)
140+
141+
#
142+
return r.zero
143+
end
144+
122145
##==============================================================================
123146
## Deprecate code below before v0.35
124147
##==============================================================================

src/Factors/EuclidDistance.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ $(TYPEDEF)
66
77
Default linear offset between two scalar variables.
88
"""
9-
struct EuclidDistance{T <: SamplableBelief} <: AbstractRelativeMinimize
9+
struct EuclidDistance{T <: SamplableBelief} <: AbstractManifoldMinimize # AbstractRelativeMinimize
1010
Z::T
1111
end
1212

src/Factors/GenericFunctions.jl

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,10 @@ export ManifoldFactor
6969
# For now, `Z` is on the tangent space in coordinates at the point used in the factor.
7070
# For groups just the lie algebra
7171
# As transition it will be easier this way, we can reevaluate
72-
struct ManifoldFactor{M <: AbstractManifold, T <: SamplableBelief} <:
73-
AbstractManifoldMinimize #AbstractFactor
72+
struct ManifoldFactor{
73+
M <: AbstractManifold,
74+
T <: SamplableBelief
75+
} <: AbstractManifoldMinimize
7476
M::M
7577
Z::T
7678
end

src/Factors/GenericMarginal.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"""
44
$(TYPEDEF)
55
"""
6-
mutable struct GenericMarginal <: AbstractRelativeRoots
6+
mutable struct GenericMarginal <: AbstractManifoldMinimize # AbstractRelativeRoots
77
Zij::Array{Float64, 1}
88
Cov::Array{Float64, 1}
99
W::Array{Float64, 1}

src/Factors/LinearRelative.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ Default linear offset between two scalar variables.
1010
X_2 = X_1 + η_Z
1111
```
1212
"""
13-
struct LinearRelative{N, T <: SamplableBelief} <: AbstractRelativeMinimize
13+
struct LinearRelative{N, T <: SamplableBelief} <: AbstractManifoldMinimize # AbstractRelativeMinimize
1414
Z::T
1515
end
1616

src/ParametricUtils.jl

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,7 @@ end
515515

516516
function solveGraphParametric(
517517
fg::AbstractDFG;
518+
verbose::Bool = false,
518519
computeCovariance::Bool = true,
519520
solveKey::Symbol = :parametric,
520521
autodiff = :forward,
@@ -571,6 +572,8 @@ function solveGraphParametric(
571572
tdtotalCost = Optim.TwiceDifferentiable(gsc, initValues; autodiff = autodiff)
572573

573574
result = Optim.optimize(tdtotalCost, initValues, alg, options)
575+
!verbose ? nothing : @show(result)
576+
574577
rv = Optim.minimizer(result)
575578

576579
# optionally compute hessian for covariance
@@ -827,6 +830,7 @@ function DFG.solveGraphParametric!(
827830
init::Bool = true,
828831
solveKey::Symbol = :parametric, # FIXME, moot since only :parametric used for parametric solves
829832
initSolveKey::Symbol = :default,
833+
verbose = false,
830834
kwargs...
831835
)
832836
# make sure variables has solverData, see #1637
@@ -837,7 +841,7 @@ function DFG.solveGraphParametric!(
837841
initParametricFrom!(fg, initSolveKey; parkey=solveKey)
838842
end
839843

840-
vardict, result, varIds, Σ = solveGraphParametric(fg; kwargs...)
844+
vardict, result, varIds, Σ = solveGraphParametric(fg; verbose, kwargs...)
841845

842846
updateParametricSolution!(fg, vardict)
843847

src/services/DeconvUtils.jl

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,28 +79,34 @@ function approxDeconv(
7979
target_smpl = makeTarget(idx)
8080

8181
# TODO must first resolve hypothesis selection before unrolling them -- deferred #1096
82-
resize!(ccw.activehypo, length(hyporecipe.activehypo[2][2]))
83-
ccw.activehypo[:] = hyporecipe.activehypo[2][2]
82+
resize!(ccw.hyporecipe.activehypo, length(hyporecipe.activehypo[2][2]))
83+
ccw.hyporecipe.activehypo[:] = hyporecipe.activehypo[2][2]
8484

8585
onehypo!, _ = _buildCalcFactorLambdaSample(ccw, idx, target_smpl, measurement)
8686
#
8787

8888
# lambda with which to find best measurement values
89-
hypoObj = (tgt) -> (target_smpl .= tgt; onehypo!())
89+
function hypoObj(tgt)
90+
copyto!(target_smpl, tgt)
91+
return onehypo!()
92+
end
93+
# hypoObj = (tgt) -> (target_smpl .= tgt; onehypo!())
9094

9195
# find solution via SubArray view pointing to original memory location
9296
if fcttype isa AbstractManifoldMinimize
9397
sfidx = ccw.varidx[]
94-
target_smpl .= _solveLambdaNumericMeas(
98+
ts = _solveLambdaNumericMeas(
9599
fcttype,
96100
hypoObj,
97101
res_,
98102
measurement[idx],
99103
getVariableType(ccw.fullvariables[sfidx]), # ccw.vartypes[sfidx](),
100104
islen1,
101105
)
106+
copyto!(target_smpl, ts)
102107
else
103-
target_smpl .= _solveLambdaNumeric(fcttype, hypoObj, res_, measurement[idx], islen1)
108+
ts = _solveLambdaNumeric(fcttype, hypoObj, res_, measurement[idx], islen1)
109+
copyto!(target_smpl, ts)
104110
end
105111
end
106112

src/services/NumericalCalculations.jl

Lines changed: 6 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -69,21 +69,6 @@ function _solveLambdaNumeric(
6969
end
7070
#
7171

72-
function _solveLambdaNumeric(
73-
fcttype::Union{F, <:Mixture{N_, F, S, T}},
74-
objResX::Function,
75-
residual::AbstractVector{<:Real},
76-
u0::AbstractVector{<:Real},
77-
islen1::Bool = false,
78-
) where {N_, F <: AbstractRelativeRoots, S, T}
79-
#
80-
81-
#
82-
r = NLsolve.nlsolve((res, x) -> res .= objResX(x), u0; inplace = true) #, ftol=1e-14)
83-
84-
#
85-
return r.zero
86-
end
8772

8873
function _solveLambdaNumeric(
8974
fcttype::Union{F, <:Mixture{N_, F, S, T}},
@@ -304,7 +289,6 @@ _getusrfnc(ccwl::CommonConvWrapper{<:Mixture}) = ccwl.usrfnc!.mechanics
304289
function _buildCalcFactor(
305290
ccwl::CommonConvWrapper,
306291
smpid,
307-
# measurement_, # deprecate
308292
varParams,
309293
activehypo,
310294
)
@@ -335,7 +319,6 @@ DevNotes
335319
- TODO refactor relationship and common fields between (CCW, FMd, CPT, CalcFactor)
336320
"""
337321
function _buildCalcFactorLambdaSample(
338-
# destVarVals::AbstractVector,
339322
ccwl::CommonConvWrapper,
340323
smpid::Integer,
341324
target, # partials no longer on coordinates at this level # = view(destVarVals[smpid], ccwl.partialDims), # target = view(ccwl.varValsAll[ccwl.varidx[]][smpid], ccwl.partialDims),
@@ -350,25 +333,9 @@ function _buildCalcFactorLambdaSample(
350333

351334
# build a view to the decision variable memory
352335
varValsHypo = ccwl.varValsAll[][ccwl.hyporecipe.activehypo]
353-
# tup = tuple(varParams...)
354-
# nms = keys(ccwl.varValsAll[])[cpt_.activehypo]
355-
# varValsHypo = NamedTuple{nms,typeof(tup)}(tup)
356-
357-
# prepare fmd according to hypo selection
358-
# FIXME must refactor (memory waste) and consolidate with CCW CPT FMd CF
359-
# FIXME move up out of smpid loop and only update bare minimal fields
360-
# _fmd_ = FactorMetadata( view(fmd_.fullvariables, cpt_.activehypo),
361-
# view(fmd_.variablelist, cpt_.activehypo),
362-
# varValsHypo, #varParams, # view(fmd_.arrRef, cpt_.activehypo),
363-
# fmd_.solvefor,
364-
# fmd_.cachedata )
365-
#
336+
366337
# get the operational CalcFactor object
367338
cf = _buildCalcFactor(ccwl, smpid, varValsHypo, ccwl.hyporecipe.activehypo)
368-
# new dev work on CalcFactor
369-
# cf = CalcFactor(ccwl.usrfnc!, _fmd_, smpid,
370-
# varValsHypo)
371-
#
372339

373340
# reset the residual vector
374341
fill!(ccwl.res, 0.0) # Roots->xDim | Minimize->zDim
@@ -414,11 +381,9 @@ DevNotes
414381
- TODO perhaps consolidate perturbation with inflation or nullhypo
415382
"""
416383
function _solveCCWNumeric!(
417-
# destVarVals::AbstractVector,
418-
ccwl::Union{CommonConvWrapper{F}, CommonConvWrapper{Mixture{N_, F, S, T}}},
384+
ccwl::Union{<:CommonConvWrapper{F}, <:CommonConvWrapper{<:Mixture{N_, F, S, T}}},
419385
_slack = nothing;
420386
perturb::Real = 1e-10,
421-
# testshuffle::Bool=false,
422387
) where {N_, F <: AbstractRelative, S, T}
423388
#
424389

@@ -504,19 +469,14 @@ end
504469
#
505470

506471
function _solveCCWNumeric!(
507-
# destVarVals::AbstractVector,
508-
ccwl::Union{CommonConvWrapper{F}, CommonConvWrapper{Mixture{N_, F, S, T}}},
472+
ccwl::Union{<:CommonConvWrapper{F}, <:CommonConvWrapper{<:Mixture{N_, F, S, T}}},
509473
_slack = nothing;
510474
perturb::Real = 1e-10,
511-
# testshuffle::Bool=false,
512475
) where {N_, F <: AbstractManifoldMinimize, S, T}
513476
#
514477
# # FIXME, move this check higher and out of smpid loop
515478
# _checkErrorCCWNumerics(ccwl, testshuffle)
516479

517-
#
518-
# thrid = Threads.threadid()
519-
520480
smpid = ccwl.particleidx[]
521481
# cannot Nelder-Mead on 1dim, partial can be 1dim or more but being conservative.
522482
islen1 = length(ccwl.partialDims) == 1 || ccwl.partial
@@ -541,8 +501,7 @@ function _solveCCWNumeric!(
541501
end
542502

543503
# TODO small off-manifold perturbation is a numerical workaround only, make on-manifold requires RoME.jl #244
544-
# use all element dimensions : ==> 1:ccwl.xDim
545-
# F <: AbstractRelativeRoots && (target .+= _perturbIfNecessary(getFactorType(ccwl), length(target), perturb))
504+
# _perturbIfNecessary(getFactorType(ccwl), length(target), perturb)
546505

547506
# do the parameter search over defined decision variables using Minimization
548507
sfidx = ccwl.varidx[]
@@ -556,14 +515,9 @@ function _solveCCWNumeric!(
556515
islen1,
557516
)
558517

559-
# Check for NaNs
560-
# FIXME isnan for manifold ProductRepr
561-
# if sum(isnan.(retval)) != 0
562-
# @error "$(ccwl.usrfnc!), ccw.thrid_=$(thrid), got NaN, smpid = $(smpid), r=$(retval)\n"
563-
# return nothing
564-
# end
518+
# TBD Check for NaNs
565519

566-
# FIXME insert result back at the correct variable element location
520+
# NOTE insert result back at the correct variable element location
567521
ccwl.varValsAll[][ccwl.varidx[]][smpid] = retval
568522

569523
return nothing

test/testSpecialOrthogonalMani.jl

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ vnd = getVariableSolverData(fg, :x1)
126126
# 23Q2 default HagerZhang fails with `AssertionError: isfinite(phi_c) && isfinite(dphi_c)`, using alternate LineSearch
127127
IIF.solveGraphParametric!(
128128
fg;
129-
algorithmkwargs=(;alphaguess = LineSearches.InitialStatic(), linesearch = LineSearches.MoreThuente())
129+
algorithmkwargs=(;alphaguess = LineSearches.InitialStatic(), linesearch = LineSearches.MoreThuente()),
130+
verbose=true
130131
)
131132

132133
##

0 commit comments

Comments
 (0)