Skip to content

Commit c6d51c2

Browse files
AffieAffie
committed
Remove DFGNodeParams, make DFGFactor immutable
1 parent d7524b3 commit c6d51c2

16 files changed

+194
-160
lines changed

‎src/Deprecated.jl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
## Remove in 0.9
88
##==============================================================================
99

10+
setTimestamp!(f::FactorDataLevel1, ts::DateTime) = error("setTimestamp!(f::FactorDataLevel1, ts::DateTime) is deprecated")
11+
1012
include("../attic/GraphsDFG/GraphsDFG.jl")
1113
@reexport using .GraphsDFGs
1214

‎src/DistributedFactorGraphs.jl

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ export DFGVariable, DFGVariableSummary, SkeletonDFGVariable
9090
export DFGFactor, DFGFactorSummary, SkeletonDFGFactor
9191

9292
# Common
93-
export DFGNodeParams
9493
export getSolvable, setSolvable!, isSolvable
9594
export getInternalId
9695

‎src/entities/AbstractDFG.jl

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,6 @@ An abstract DFG factor.
2020
abstract type AbstractDFGFactor <: DFGNode
2121
end
2222

23-
"""
24-
$(TYPEDEF)
25-
The common node parameters for variables and factors.
26-
"""
27-
mutable struct DFGNodeParams
28-
solvable::Int
29-
end
3023

3124
"""
3225
$(TYPEDEF)

‎src/entities/DFGFactor.jl

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ Complete factor structure for a DistributedFactorGraph factor.
9494
Fields:
9595
$(TYPEDFIELDS)
9696
"""
97-
mutable struct DFGFactor{T} <: AbstractDFGFactor
97+
struct DFGFactor{T, N} <: AbstractDFGFactor
9898
"""Factor label, e.g. :x1f1.
9999
Accessor: [`getLabel`](@ref)"""
100100
label::Symbol
@@ -106,17 +106,23 @@ mutable struct DFGFactor{T} <: AbstractDFGFactor
106106
tags::Set{Symbol}
107107
"""Solver data.
108108
Accessors: [`getSolverData`](@ref), [`setSolverData!`](@ref)"""
109-
solverData::GenericFunctionNodeData{T}
109+
solverData::Base.RefValue{GenericFunctionNodeData{T}}
110110
"""Solvable flag for the factor.
111-
Accessors: [`getSolvable`](@ref), [`setSolvable!`](@ref)
112-
TODO: Switch to `DFGNodeParams`"""
113-
solvable::Int #TODO we can go with DFGNodeParams or Reference the solvable field with getproperty overload
114-
"""Mutable parameters for the variable. We suggest using accessors to get to this data.
115111
Accessors: [`getSolvable`](@ref), [`setSolvable!`](@ref)"""
116-
_dfgNodeParams::DFGNodeParams
112+
solvable::Base.RefValue{Int}
117113
"""Internal cache of the ordering of the neighbor variables. Rather use getVariableOrder to get the list as this is an internal value.
118114
Accessors: [`getVariableOrder`](@ref)"""
119-
_variableOrderSymbols::Vector{Symbol}
115+
_variableOrderSymbols::NTuple{N,Symbol}
116+
117+
# Inner constructor
118+
DFGFactor{T}(label::Symbol,
119+
timestamp::DateTime,
120+
tags::Set{Symbol},
121+
solverData::GenericFunctionNodeData{T},
122+
solvable::Int,
123+
_variableOrderSymbols::NTuple{N,Symbol}) where {T,N} =
124+
new{T,N}(label, timestamp, tags, Ref(solverData), Ref(solvable), _variableOrderSymbols)
125+
120126
end
121127

122128
##------------------------------------------------------------------------------
@@ -127,8 +133,17 @@ $(SIGNATURES)
127133
128134
Construct a DFG factor given a label.
129135
"""
130-
DFGFactor{T}(label::Symbol, timestamp::DateTime=now()) where {T} =
131-
DFGFactor(label, timestamp, Set{Symbol}(), GenericFunctionNodeData{T}(), 1, DFGNodeParams(1), Symbol[])
136+
DFGFactor(label::Symbol,
137+
timestamp::DateTime,
138+
tags::Set{Symbol},
139+
solverData::GenericFunctionNodeData{T},
140+
solvable::Int,
141+
_variableOrderSymbols::Tuple) where {T} =
142+
DFGFactor{T}(label, timestamp, tags, solverData, solvable, _variableOrderSymbols)
143+
144+
145+
DFGFactor{T}(label::Symbol, variableOrderSymbols::Vector{Symbol}, timestamp::DateTime=now(), data::GenericFunctionNodeData{T} = GenericFunctionNodeData{T}()) where {T} =
146+
DFGFactor(label, timestamp, Set{Symbol}(), data, 1, Tuple(variableOrderSymbols))
132147

133148

134149
DFGFactor(label::Symbol,
@@ -137,22 +152,23 @@ DFGFactor(label::Symbol,
137152
tags::Set{Symbol}=Set{Symbol}(),
138153
timestamp::DateTime=now(),
139154
solvable::Int=1) where {T} =
140-
DFGFactor{T}(label,timestamp,tags,data,solvable,DFGNodeParams(solvable),variableOrderSymbols)
155+
DFGFactor{T}(label, timestamp, tags, data, solvable, Tuple(variableOrderSymbols))
141156

142157

143158

144159
Base.getproperty(x::DFGFactor,f::Symbol) = begin
145-
if f == :solvable
146-
getfield(x,:_dfgNodeParams).solvable
160+
if f == :solvable || f == :solverData
161+
getfield(x, f)[]
162+
elseif f == :_variableOrderSymbols
163+
[getfield(x,f)...]
147164
else
148165
getfield(x,f)
149166
end
150167
end
151168

152169
Base.setproperty!(x::DFGFactor,f::Symbol, val) = begin
153-
if f == :solvable
154-
setfield!(x,f,val)
155-
getfield(x,:_dfgNodeParams).solvable = val
170+
if f == :solvable || f == :solverData
171+
getfield(x,f)[] = val
156172
else
157173
setfield!(x,f,val)
158174
end

‎src/entities/DFGVariable.jl

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -200,9 +200,9 @@ struct DFGVariable{T<:InferenceVariable} <: AbstractDFGVariable
200200
"""Dictionary of large data associated with this variable.
201201
Accessors: [`addBigDataEntry!`](@ref), [`getBigDataEntry`](@ref), [`updateBigDataEntry!`](@ref), and [`deleteBigDataEntry!`](@ref)"""
202202
bigData::Dict{Symbol, AbstractBigDataEntry}
203-
"""Mutable parameters for the variable. We suggest using accessors to get to this data.
203+
"""Solvable flag for the variable.
204204
Accessors: [`getSolvable`](@ref), [`setSolvable!`](@ref)"""
205-
_dfgNodeParams::DFGNodeParams
205+
solvable::Base.RefValue{Int}
206206
end
207207

208208
##------------------------------------------------------------------------------
@@ -220,7 +220,7 @@ DFGVariable(label::Symbol, softtype::T;
220220
smallData::Dict{String, String}=Dict{String, String}(),
221221
bigData::Dict{Symbol, AbstractBigDataEntry}=Dict{Symbol,AbstractBigDataEntry}(),
222222
solvable::Int=1) where {T <: InferenceVariable} =
223-
DFGVariable{T}(label, timestamp, tags, estimateDict, solverDataDict, smallData, bigData, DFGNodeParams(solvable))
223+
DFGVariable{T}(label, timestamp, tags, estimateDict, solverDataDict, smallData, bigData, Ref(solvable))
224224

225225

226226
DFGVariable(label::Symbol,
@@ -231,19 +231,19 @@ DFGVariable(label::Symbol,
231231
smallData::Dict{String, String}=Dict{String, String}(),
232232
bigData::Dict{Symbol, AbstractBigDataEntry}=Dict{Symbol,AbstractBigDataEntry}(),
233233
solvable::Int=1) where {T <: InferenceVariable} =
234-
DFGVariable{T}(label, timestamp, tags, estimateDict, Dict{Symbol, VariableNodeData{T}}(:default=>solverData), smallData, bigData, DFGNodeParams(solvable))
234+
DFGVariable{T}(label, timestamp, tags, estimateDict, Dict{Symbol, VariableNodeData{T}}(:default=>solverData), smallData, bigData, Ref(solvable))
235235

236236
Base.getproperty(x::DFGVariable,f::Symbol) = begin
237237
if f == :solvable
238-
getfield(x,:_dfgNodeParams).solvable
238+
getfield(x,f)[]
239239
else
240240
getfield(x,f)
241241
end
242242
end
243243

244244
Base.setproperty!(x::DFGVariable,f::Symbol, val) = begin
245245
if f == :solvable
246-
getfield(x,:_dfgNodeParams).solvable = val
246+
getfield(x,f)[] = val
247247
else
248248
setfield!(x,f,val)
249249
end

‎src/services/AbstractDFG.jl

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -379,12 +379,18 @@ $(SIGNATURES)
379379
"""
380380
function addFactor!(dfg::AbstractDFG, variables::Vector{<:AbstractDFGVariable}, factor::F)::F where F <: AbstractDFGFactor
381381

382-
variableLabels = map(v->v.label, variables)
382+
Base.depwarn("addFactor!(dfg, variables, factor) is deprecated, use addFactor!(dfg, factor)", :addFactor!)
383+
variableLabels = map(v->v.label, variables)
383384

385+
if factor isa DFGFactor
386+
f = factor
387+
newfactor = DFGFactor(f.label, f.timestamp, f.tags, f.solverData, f.solvable, Tuple(variableLabels))
388+
return addFactor!(dfg, newfactor)
389+
else
384390
resize!(factor._variableOrderSymbols, length(variableLabels))
385391
factor._variableOrderSymbols .= variableLabels
386-
387392
return addFactor!(dfg, factor)
393+
end
388394

389395
end
390396

@@ -393,10 +399,18 @@ $(SIGNATURES)
393399
"""
394400
function addFactor!(dfg::AbstractDFG, variableLabels::Vector{Symbol}, factor::F)::AbstractDFGFactor where F <: AbstractDFGFactor
395401

396-
resize!(factor._variableOrderSymbols, length(variableLabels))
397-
factor._variableOrderSymbols .= variableLabels
402+
Base.depwarn("addFactor!(dfg, variables, factor) is deprecated, use addFactor!(dfg, factor)", :addFactor!)
403+
404+
if factor isa DFGFactor
405+
f = factor
406+
newfactor = DFGFactor(f.label, f.timestamp, f.tags, f.solverData, f.solvable, Tuple(variableLabels))
407+
return addFactor!(dfg, newfactor)
408+
else
409+
resize!(factor._variableOrderSymbols, length(variableLabels))
410+
factor._variableOrderSymbols .= variableLabels
411+
return addFactor!(dfg, factor)
412+
end
398413

399-
return addFactor!(dfg, factor)
400414
end
401415

402416

‎src/services/CommonAccessors.jl

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ Variables or factors may or may not be 'solvable', depending on a user definitio
8484
Related:
8585
- isSolveInProgress
8686
"""
87-
getSolvable(var::Union{DFGVariable, DFGFactor})::Int = var._dfgNodeParams.solvable
87+
getSolvable(var::Union{DFGVariable, DFGFactor})::Int = var.solvable
8888
#TODO DataLevel2
8989

9090
"""
@@ -94,9 +94,9 @@ Get 'solvable' parameter for either a variable or factor.
9494
"""
9595
function getSolvable(dfg::AbstractDFG, sym::Symbol)::Int
9696
if isVariable(dfg, sym)
97-
return getVariable(dfg, sym)._dfgNodeParams.solvable
97+
return getVariable(dfg, sym).solvable
9898
elseif isFactor(dfg, sym)
99-
return getFactor(dfg, sym)._dfgNodeParams.solvable
99+
return getFactor(dfg, sym).solvable
100100
end
101101
end
102102

@@ -107,7 +107,7 @@ end
107107
Set the `solvable` parameter for either a variable or factor.
108108
"""
109109
function setSolvable!(node::N, solvable::Int)::Int where N <: DFGNode
110-
node._dfgNodeParams.solvable = solvable
110+
node.solvable = solvable
111111
return solvable
112112
end
113113

@@ -119,9 +119,9 @@ Set the `solvable` parameter for either a variable or factor.
119119
"""
120120
function setSolvable!(dfg::AbstractDFG, sym::Symbol, solvable::Int)::Int
121121
if isVariable(dfg, sym)
122-
getVariable(dfg, sym)._dfgNodeParams.solvable = solvable
122+
getVariable(dfg, sym).solvable = solvable
123123
elseif isFactor(dfg, sym)
124-
getFactor(dfg, sym)._dfgNodeParams.solvable = solvable
124+
getFactor(dfg, sym).solvable = solvable
125125
end
126126
return solvable
127127
end

‎src/services/CompareUtils.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ implement compare if needed.
1818

1919

2020
# Generate compares automatically for all in this union
21-
const GeneratedCompareUnion = Union{MeanMaxPPE, VariableNodeData, DFGNodeParams,
21+
const GeneratedCompareUnion = Union{MeanMaxPPE, VariableNodeData,
2222
DFGVariable, DFGVariableSummary, SkeletonDFGVariable,
2323
GenericFunctionNodeData,
2424
DFGFactor, DFGFactorSummary, SkeletonDFGFactor}

‎src/services/CustomPrinting.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ function printVariable(io::IO, vert::DFGVariable;
5252

5353
for f in fields
5454
printstyled(ioc, f,":\n", color=:blue)
55-
show(ioc, getfield(vert, f))
55+
show(ioc, getproperty(vert, f))
5656
println(ioc)
5757
end
5858
end
@@ -86,7 +86,7 @@ function printFactor(io::IO, vert::DFGFactor;
8686

8787
for f in fields
8888
printstyled(ioc, f,":\n", color=:blue)
89-
show(ioc, getfield(vert, f))
89+
show(ioc, getproperty(vert, f))
9090
println(ioc)
9191
end
9292
end

‎src/services/DFGFactor.jl

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,13 @@ end
6969

7070

7171
function setTimestamp(f::DFGFactor, ts::DateTime)
72-
return DFGFactor(f.label, ts, f.tags, f.solverData, f.solvable, f._dfgNodeParams, f._variableOrderSymbols)
72+
return DFGFactor(f.label, ts, f.tags, f.solverData, f.solvable, getfield(f,:_variableOrderSymbols))
7373
end
7474

7575
function setTimestamp(f::DFGFactorSummary, ts::DateTime)
7676
return DFGFactorSummary(f.label, ts, f.tags, f._variableOrderSymbols)
7777
end
7878

79-
setTimestamp!(f::FactorDataLevel1, ts::DateTime) = f.timestamp = ts
80-
8179

8280
##------------------------------------------------------------------------------
8381
## solvable
@@ -89,7 +87,7 @@ setTimestamp!(f::FactorDataLevel1, ts::DateTime) = f.timestamp = ts
8987
# isSolvable
9088

9189
##------------------------------------------------------------------------------
92-
## _dfgNodeParams [solvable]
90+
## solvable
9391
##------------------------------------------------------------------------------
9492

9593
## COMMON
@@ -122,7 +120,6 @@ function getSolverData(f::F) where F <: DFGFactor
122120
return f.solverData
123121
end
124122

125-
#TODO don't know if this is used, added for completeness
126123
setSolverData!(f::DFGFactor, data::GenericFunctionNodeData) = f.solverData = data
127124

128125

0 commit comments

Comments
 (0)