Rename FactorDFG.data / fnctype becomes...

[dehann]

@Affie and I had a long frustrating conversation to confirm what we already know: That factor data fields need to be updated for DFG 1 milestone. This ticket is the confluence of many other comments including the most recent:

• Dont nest JSON strings in pre JSON structs (FactorDFG) #1118

Context for larger work beyond this issue:

1. [This issue] Add new field to FactorDFG.observFnc, and replace associated old accessors such as @deprecate getObservationFunction(::FactorDFG) getFactorFunction(::FactorCompute).
   TAC

• spike to check no adverse effect on SDK.rs/c
• add new field to FactorDFG.observFnc (side by side with old .data / .fnctype)
  • Also see performance discussion (or Summary level) FactorDFG.observFnc_type::String Investigate splitting factor operational part from 'constant' part of factor #992
• implement getObservationFunction(::FactorDFG)
• deprecate getFactorType use getObservationFunction( DFG or Compute )

2. Move all other FunctionNodeData fields to FactorDFG - Investigate splitting factor operational part from 'constant' part of factor #992
3. Update CCW construction when building FactorCompute - Investigate splitting factor operational part from 'constant' part of factor #992

---

Additional links from discussion:

• FactorDFG.data refactor concept discussion #1120

[dehann]

Assigned to me for spike on impact on current Rust SDK work.

[Affie]

Also related and blocking the current factor serialization design: #1077

[Affie]

Is the Function part needed in getObservationFunction or will it be clear enough to only use getObservation?

I'm leaning towards only Observation as noun.

Is this definition accurate:
An observation is a piece of data or measurement obtained from the real world, typically through sensors or other data collection methods, which is used to inform and update the state of a factor graph. Observations are used to create factors that encode the relationship between variables in the graph, helping to constrain and refine the estimates of those variables.

We also have getMeasurementParametric that only extracts the parametric data part.

[dehann]

Hi @Affie , i think we discussed a lot of this earlier today. I'm going to look into if SDK.rs has issues with JSON (or at least how to move that forward, but after that we may need to reconnect on what we want to do with this task).

[Affie]

after that we may need to reconnect on what we want to do with this task

I only know of 2 outstanding design aspects

1. Rename FactorDFG.data / fnctype becomes... #1119 (comment)
2. Long-term plans for the FactorDFG observation function field type. My current best guess is that it will remain as a JSON serialized string.

[dehann]

I'm leaning towards only Observation as noun.

Yes, sounds good, So there will be observation and state fields in factor.

Is the Function part needed in getObservationFunction or will it be clear enough to only use getObservation?

the shortened one looks fine to me.

[dehann]

Unassigning from me and moved to Triage -- I finished the spike on JSONCRUD for SDK.rs and we can work around graphql_client hard-types with new callback approach for splicing in rust's serde_json::{Map,Value}. So think this is a go.

[dehann]

Hi @Affie think you are busy with these tasks anyway?

[Affie]

Yes I think it is easiest to do the rafactor as part of #1127

[dehann]

sounds like two questions:

• rename field to observation?
• should we parameterize FactorDFG{T}, whats going on in SDK.rs?

[Affie]

should we parameterize FactorDFG{T}, whats going on in SDK.rs?

I'm leaning towards keeping FactorDFG as is but I'm open to changing it if there are good reasons.
We have a "factor type" field for T and non-parameterized makes usage and serialization simpler.