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.

Sign up for GitHub

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

Jump to bottom

Simple copy functions #570

Closed
wants to merge 1 commit into from
Closed

Simple copy functions #570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions src/DistributedFactorGraphs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ export listTags, mergeTags!, removeTags!, emptyTags!
#this isn't acttually implemented. TODO remove or implement
export addTags!

import Base: copy
export copy

##------------------------------------------------------------------------------
# Variable
##------------------------------------------------------------------------------
Expand Down
40 changes: 31 additions & 9 deletions src/entities/DFGVariable.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,28 @@ VariableNodeData(val::Array{Float64,2},
VariableNodeData(softtype::T; solverKey::Symbol=:default) where T <: InferenceVariable =
VariableNodeData{T}(zeros(1,1), zeros(1,1), Symbol[], Int[], 0, false, :NOTHING, Symbol[], softtype, false, 0.0, false, false, 0, 0, solverKey)

## Copying
function copy(vnd::VariableNodeData; solverKey::Union{Nothing,Symbol}=nothing)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we shouldn't overwrite expected behaviour from base copy with deepcopy?
I expect copy to return a shallow copy and deepcopy for this.

Personally I don't think we should provide too many functions. Every function has to be documented and maintained.
The user can rather just do

dcvnd = deepcopy(vnd)
dcvnd.selverKey = newKey

Is the function we are trying to replace updateSolverData!(dfg, label, vnd, newsolvekey)?
Where vnd.solvekey != newsolvekey.
Perhaps something like (although IIF needs to be updated slightly for it):

function deepcopySolverData!(dfg::AbstractDFG, label::Symbol, srcKey::Sybol, dstKey::Symbol)
    vnd = deepcopy(getSolverData!(dfg, label, srcKey))
    vnd.solverKey = dstKey
    updateSolverData!(dfg, vnd)
end

Now that I'm thinking it through. update is maybe not the correct function to use. Its adding new solverdata so maybe:

function addSolverData!(dfg::AbstractDFG, label::Symbol, vnd, solverKey::Symbol)
    vnd.solverKey = dstKey
    addSolverData!(dfg, vnd)
end

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cool, that works, will close this then.

return VariableNodeData(
deepcopy(vnd.val),
deepcopy(vnd.bw),
deepcopy(vnd.BayesNetOutVertIDs),
deepcopy(vnd.dimIDs),
vnd.dims,
vnd.eliminated,
vnd.BayesNetVertID,
deepcopy(vnd.separator),
vnd.softtype,
vnd.initialized,
vnd.inferdim,
vnd.ismargin,
vnd.dontmargin,
vnd.solveInProgress,
vnd.solvedCount,
solverKey == nothing ? vnd.solverKey : solverKey)
end


##==============================================================================
## PackedVariableNodeData.jl
##==============================================================================
Expand Down Expand Up @@ -194,6 +216,15 @@ end

MeanMaxPPE(solverKey::Symbol, suggested::Vector{Float64}, max::Vector{Float64}, mean::Vector{Float64}) = MeanMaxPPE(solverKey, suggested, max, mean, now(UTC))

## Copying
function copy(ppe::MeanMaxPPE; solverKey::Union{Nothing,Symbol}=nothing)
return MeanMaxPPE(solverKey == nothing ? ppe.solverKey : solverKey,
ppe.suggested,
ppe.max,
ppe.mean,
ppe.lastUpdatedTimestamp)
end

## Metadata
"""
$SIGNATURES
Expand Down Expand Up @@ -315,15 +346,6 @@ Base.setproperty!(x::DFGVariable,f::Symbol, val) = begin
end
end


##------------------------------------------------------------------------------
# TODO: can't see the reason to overwrite copy, leaving it here for now
# function Base.copy(o::DFGVariable)::DFGVariable
# return DFGVariable(o.label, getSofttype(o)(), tags=copy(o.tags), estimateDict=copy(o.estimateDict),
# solverDataDict=copy(o.solverDataDict), smallData=copy(o.smallData),
# dataDict=copy(o.dataDict), solvable=getSolvable(o))
# end

##------------------------------------------------------------------------------
## DFGVariableSummary lv1
##------------------------------------------------------------------------------
Expand Down