Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

variable solver data concept #231

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/entities/DFGFactor.jl
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,14 @@ end
"""
$(TYPEDEF)
Skeleton factor with essentials.
$(FIELDS)
"""
struct SkeletonDFGFactor <: AbstractDFGFactor
"Factor label, :x1f1, `label(factor)`"
label::Symbol
"Factor tags, [:FACTOR], `tags(factor)`"
tags::Vector{Symbol}
"The order of the variables associated with this factor."
_variableOrderSymbols::Vector{Symbol}
end

Expand Down
76 changes: 76 additions & 0 deletions src/services/AbstractDFG.jl
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,82 @@ function _copyIntoGraph!(sourceDFG::G, destDFG::H, variableFactorLabels::Vector{
return nothing
end

#TODO UNTESTED and NOT FILLED IN, just to define function signatures
"""
$(SIGNATURES)
"""
function getVariableSolverData(dfg::AbstractDFG, variablekey::Symbol, solvekey::Symbol=:default)
error("not implemented")
return deepcopy(solverData(getVariable(dfg, variablekey), solvekey))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why a deepcopy?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should verb get not return the original memory object?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think you are correct.

end

"""
$(SIGNATURES)
"""
function addVariableSolverData!(dfg::AbstractDFG, vnd::VariableNodeData, variablekey::Symbol, solvekey::Symbol=:default)
#for InMemoryDFGTypes, cloud would update here
error("not implemented")

var = getVariable(dfg, variablekey)

if haskey(var.solverDataDict, solvekey)
error("VariableNodeData '$(solvekey)' already exists")
end

#var.solverDataDict[solvekey] = vnd

setSolverData(var, deepcopy(vnd), key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see updated api in master to include an !


end

addVariableSolverData!(dfg::AbstractDFG, sourceVariable::DFGVariable, solvekey::Symbol=:default) =
addVariableSolverData!(dfg, solverData(sourceVariable, solvekey), sourceVariable.label, solvekey)


"""
$(SIGNATURES)
"""
function updateVariableSolverData!(dfg::AbstractDFG, vnd::VariableNodeData, variablekey::Symbol, solvekey::Symbol=:default)
error("not implemented")
#This is basically just setSolverData with a copy
var = getVariable(dfg, variablekey)

#var.solverDataDict[solvekey] = deepcopy(vnd)
#for InMemoryDFGTypes, cloud would update here
setSolverData(var, deepcopy(vnd), key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, why do a deep copy here? You think the user might accidentally point the code to the same vnd object and therefore become incorrect?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should maybe leave the ‘deepcopy’ responsibility to the user.

end

updateVariableSolverData!(dfg::AbstractDFG, sourceVariable::DFGVariable, solvekey::Symbol=:default) =
updateVariableSolverData!(dfg, solverData(sourceVariable, solvekey), sourceVariable.label, solvekey)

function updateVariableSolverData!(dfg::AbstractDFG, sourceVariables::Vector{DFGVariable}, solvekey::Symbol=:default)
#I think cloud would do this in bulk for speed
for var in sourceVariables
updateVariableSolverData!(dfg, solverData(var, solvekey), var.label, solvekey)
end
end

"""
$(SIGNATURES)
"""
function deleteVariableSolverData!(dfg::AbstractDFG, variablekey::Symbol, solvekey::Symbol=:default)
error("not implemented")

var = getVariable(dfg, variablekey)

if haskey(var.solverDataDict, solvekey)
error("VariableNodeData '$(solvekey)' already exists")
end

vnd = pop!(var.solverDataDict, solvekey)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one seems right to me, will think on it more.


return vnd
end

deleteVariableSolverData!(dfg::AbstractDFG, sourceVariable::DFGVariable, solvekey::Symbol=:default) =
deleteVariableSolverData!(dfg, sourceVariable.label, solvekey)


"""
$(SIGNATURES)
Merges and updates solver and estimate data for a variable (variable can be from another graph).
Expand Down