updateVariableSolverData! bug and API

[Affie]

MethodError: no method matching updateVariableSolverData!(::LightDFG{SolverParams,DFGVariable,DFGFactor}, ::Symbol, ::Bool, ::Array{Symbol,1}, ::Bool)

and

MethodError: no method matching  updateVariableSolverData!(::LightDFG{SolverParams,DFGVariable,DFGFactor}, ::Symbol, ::Nothing, ::Symbol, ::Bool)

and this is also broken:

updateVariableSolverData!(dfg, xi.label, getSolverData(xi, :default), :graphinit, true, Symbol[], false)

[Affie]

Add tests for this in DFG

[Affie]

DistributedFactorGraphs.jl/src/services/DFGVariable.jl

Line 524 in 21655bf

@warn "TODO It looks like solverKey as parameter is deprecated, set it in vnd, or keep this function?"

We should decide how we want to use this.

[Affie]

originally posted in a comment: #570 (comment)

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  

[dehann]

Maybe we shouldn't overwrite expected behaviour from base copy with deepcopy?

definitely not change the behaviour of a method from Base.

[dehann]

Is the function we are trying to replace updateSolverData!(dfg, label, vnd, newsolvekey)?

No, the idea is just to have the solveKey available inside the structure as a duplicate from the Dict container in which the VND is stored. Nothing crazy, just duplication.

Perhaps something like (although IIF needs to be updated slightly for it):

I disagree with the user having to do band-aids for DFG internal duplication decision.

[dehann]

update is maybe not the correct function to use. Its adding new solverdata so maybe:

update verb will need to update both registers of the duplication.

[Affie]

No, the idea is just to have the solveKey available inside the structure as a duplicate from the Dict container in which the VND is stored. Nothing crazy, just duplication.

Dehann, I think you are missing the point

[dehann]

maybe, how I understand it is that Sam asked for a duplicate of the solveKey inside VND. So nothing changes, except that there is a forced duplication of one ::Symbol register.

Dict{Symbol, VND}( mySolveKey => VND(... solveKey=mySolveKey))