Refactoring Factors: data -> state, observation and operational memory #1127
Conversation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* defFactorFunction macro * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * update defFactorType * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Update DFGFactor.jl --------- Co-authored-by: Johannes Terblanche <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| :bcf1, | ||
| [:b, :c], | ||
| ZonedDateTime("2020-08-11T00:12:03.000-05:00"), | ||
| GenericFunctionNodeData(; fnc=TestCCW{TestFunctorInferenceType1}()); |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
| GenericFunctionNodeData(; fnc=TestCCW{TestFunctorInferenceType1}()); | |
| GenericFunctionNodeData(; fnc = TestCCW{TestFunctorInferenceType1}()); |
| @@ -473,7 +473,7 @@ function VariablesandFactorsCRUD_SET!(fg, v1, v2, v3, f0, f1, f2) | |||
| @test getLabel(fg[getLabel(v1)]) == getLabel(v1) | |||
|
|
|||
| #TODO standardize this error and res also for that matter | |||
| fnope = FactorCompute{TestCCW{TestFunctorInferenceType1}}(:broken, [:a, :nope]) | |||
| fnope = FactorCompute(:broken, [:a, :nope], GenericFunctionNodeData(; fnc=TestCCW{TestFunctorInferenceType1}())) | |||
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
| fnope = FactorCompute(:broken, [:a, :nope], GenericFunctionNodeData(; fnc=TestCCW{TestFunctorInferenceType1}())) | |
| fnope = FactorCompute( | |
| :broken, | |
| [:a, :nope], | |
| GenericFunctionNodeData(; fnc = TestCCW{TestFunctorInferenceType1}()), | |
| ) |
| @@ -1683,7 +1683,7 @@ function ProducingDotFiles( | |||
| end | |||
| if f1 === nothing | |||
| if (FACTYPE == FactorCompute) | |||
| f1 = FactorCompute{TestFunctorInferenceType1}(:abf1, [:a, :b]) | |||
| f1 = FactorCompute(:abf1, [:a, :b], GenericFunctionNodeData(;fnc = TestFunctorInferenceType1())) | |||
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
| f1 = FactorCompute(:abf1, [:a, :b], GenericFunctionNodeData(;fnc = TestFunctorInferenceType1())) | |
| f1 = FactorCompute( | |
| :abf1, | |
| [:a, :b], | |
| GenericFunctionNodeData(; fnc = TestFunctorInferenceType1()), | |
| ) |
| Symbol("x$(n)x$(n+1)f1"), | ||
| [verts[n].label, verts[n + 1].label], | ||
| GenericFunctionNodeData(; fnc=TestFunctorInferenceType1()) |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
| GenericFunctionNodeData(; fnc=TestFunctorInferenceType1()) | |
| GenericFunctionNodeData(; fnc = TestFunctorInferenceType1()), |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1127 +/- ##
===========================================
- Coverage 75.32% 74.81% -0.51%
===========================================
Files 28 28
Lines 2399 2450 +51
===========================================
+ Hits 1807 1833 +26
- Misses 592 617 +25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| data::GenericFunctionNodeData{T} = GenericFunctionNodeData(; fnc = T()); | ||
| kw..., | ||
| ) where {T} | ||
| solverData = nothing |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
| solverData = nothing | |
| solverData = nothing, |
| Base.depwarn( | ||
| "`FactorCompute` solverData is deprecated", | ||
| :FactorCompute, | ||
| ) |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
| Base.depwarn( | |
| "`FactorCompute` solverData is deprecated", | |
| :FactorCompute, | |
| ) | |
| Base.depwarn("`FactorCompute` solverData is deprecated", :FactorCompute) |
| ) | ||
| refsolverData = Ref(solverData) | ||
| end | ||
|
|
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
| """Temporary, non-persistent memory used internally by the solver for intermediate numerical computations and buffers. | ||
| `workmem` is lazily allocated and only used during factor operations; it is not serialized or retained after solving. | ||
| Accessors: [`getWorkmem`](@ref), [`setWorkmem!`](@ref)""" | ||
| workmem::Base.RefValue{<:FactorOperationalMemory} #TODO easy of use vs. performance as container is abstract in any case. |
There was a problem hiding this comment.
@dehann, I would like some input on the naming here. I asked copilot and it came up with Workmem (working memory). I came up with Opmem (operational memory). As you know the only implementation here is CommonConvWrapper.
FactorOperationalMemory can also be renamed to FactorWorkingMemory if it is clearer.
A confusing name such as rebuildFactorMetadata! will then become rebuildFactorWorkmem since Metadata has another meaning.
There was a problem hiding this comment.
I don't think it is part of the SDKs and likely only for advanced (solver) devs.
So far this is only a spike as part of experimenting with the #992 design.
PR should do:
xref: