Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
Affie and github-actions[bot] authored Nov 26, 2024
1 parent d0c259f commit ee12b8e
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/DistributedFactorGraphs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ include("weakdeps_prototypes.jl")
export SkeletonDFGVariable
const SkeletonDFGVariable = VariableSkeleton

export DFGVariableSummary
export DFGVariableSummary
const DFGVariableSummary = VariableSummary

export DFGVariable
Expand Down
6 changes: 4 additions & 2 deletions src/FileDFG/services/FileDFG.jl
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ function loadDFG!(

# FIXME, why is this treated different from VariableSkeleton, VariableSummary?

usePackedVariable = isa(dfgLoadInto, GraphsDFG) && getTypeDFGVariables(dfgLoadInto) == VariableDFG
usePackedVariable =
isa(dfgLoadInto, GraphsDFG) && getTypeDFGVariables(dfgLoadInto) == VariableDFG
# type instability on `variables` as either `::Vector{Variable}` or `::Vector{VariableCompute{<:}}` (vector of abstract)
variables = @showprogress 1 "loading variables" asyncmap(varFiles) do varFile
jstr = read("$varFolder/$varFile", String)
Expand All @@ -179,7 +180,8 @@ function loadDFG!(
# Adding variables
map(v -> addVariable!(dfgLoadInto, v), variables)

usePackedFactor = isa(dfgLoadInto, GraphsDFG) && getTypeDFGFactors(dfgLoadInto) == FactorDFG
usePackedFactor =
isa(dfgLoadInto, GraphsDFG) && getTypeDFGFactors(dfgLoadInto) == FactorDFG

# `factors` is not type stable `::Vector{Factor}` or `::Vector{FactorCompute{<:}}` (vector of abstract)
factors = @showprogress 1 "loading factors" asyncmap(factorFiles) do factorFile
Expand Down
12 changes: 10 additions & 2 deletions src/GraphsDFG/entities/GraphsDFG.jl
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,22 @@ end

# GraphsDFG{T}(; kwargs...) where T <: AbstractParams = GraphsDFG{T,VariableCompute,FactorCompute}(;kwargs...)
function GraphsDFG{T}(
g::FactorGraph{Int, VariableCompute, FactorCompute} = FactorGraph{Int, VariableCompute, FactorCompute}();
g::FactorGraph{Int, VariableCompute, FactorCompute} = FactorGraph{
Int,
VariableCompute,
FactorCompute,
}();
kwargs...,
) where {T <: AbstractParams}
return GraphsDFG{T, VariableCompute, FactorCompute}(g; kwargs...)
end

function GraphsDFG(
g::FactorGraph{Int, VariableCompute, FactorCompute} = FactorGraph{Int, VariableCompute, FactorCompute}();
g::FactorGraph{Int, VariableCompute, FactorCompute} = FactorGraph{
Int,
VariableCompute,
FactorCompute,
}();
solverParams::T = NoSolverParams(),
kwargs...,
) where {T}
Expand Down
6 changes: 5 additions & 1 deletion src/entities/DFGVariable.jl
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,11 @@ function VariableCompute(label::Symbol, variableType::InferenceVariable; kwargs.
end

function VariableCompute(label::Symbol, solverData::VariableNodeData; kwargs...)
return VariableCompute(; label, solverDataDict = Dict(:default => solverData), kwargs...)
return VariableCompute(;
label,
solverDataDict = Dict(:default => solverData),
kwargs...,
)
end

Base.getproperty(x::VariableCompute, f::Symbol) = begin
Expand Down
10 changes: 8 additions & 2 deletions src/services/CommonAccessors.jl
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,10 @@ Related
isSolvable
"""
function getSolveInProgress(var::Union{VariableCompute, FactorCompute}, solveKey::Symbol = :default)
function getSolveInProgress(
var::Union{VariableCompute, FactorCompute},
solveKey::Symbol = :default,
)
# Variable
if var isa VariableCompute
if haskey(getSolverDataDict(var), solveKey)
Expand All @@ -170,7 +173,10 @@ end

#TODO missing set solveInProgress and graph level accessor

function isSolveInProgress(node::Union{VariableCompute, FactorCompute}, solvekey::Symbol = :default)
function isSolveInProgress(
node::Union{VariableCompute, FactorCompute},
solvekey::Symbol = :default,
)
return getSolveInProgress(node, solvekey) > 0
end

Expand Down
11 changes: 9 additions & 2 deletions src/services/DFGVariable.jl
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,10 @@ Merges and updates solver and estimate data for a variable (variable can be from
If the same key is present in another collection, the value for that key will be the value it has in the last collection listed (updated).
Note: Makes a copy of the estimates and solver data so that there is no coupling between graphs.
"""
function mergeVariableSolverData!(destVariable::VariableCompute, sourceVariable::VariableCompute)
function mergeVariableSolverData!(
destVariable::VariableCompute,
sourceVariable::VariableCompute,
)
# We don't know which graph this came from, must be copied!
merge!(destVariable.solverDataDict, deepcopy(sourceVariable.solverDataDict))
return destVariable
Expand Down Expand Up @@ -982,7 +985,11 @@ end
Add a new PPE entry from a deepcopy of the source variable PPE.
NOTE: Copies the PPE.
"""
function addPPE!(dfg::AbstractDFG, sourceVariable::VariableCompute, ppekey::Symbol = :default)
function addPPE!(
dfg::AbstractDFG,
sourceVariable::VariableCompute,
ppekey::Symbol = :default,
)
return addPPE!(dfg, sourceVariable.label, deepcopy(getPPE(sourceVariable, ppekey)))
end

Expand Down
4 changes: 3 additions & 1 deletion test/compareTests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ v3 = VariableCompute(:x2, TestVariableType2())
v2.solvable = 0
@test !(v1 == v2)
@test !(v1 == v3)
@test !(VariableCompute(:x1, TestVariableType1()) == VariableCompute(:x1, TestVariableType2()))
@test !(
VariableCompute(:x1, TestVariableType1()) == VariableCompute(:x1, TestVariableType2())
)

# GenericFunctionNodeData
gfnd1 = GenericFunctionNodeData(;
Expand Down

0 comments on commit ee12b8e

Please sign in to comment.