FhSim  3.1.0
Marine systems simulation
Loading...
Searching...
No Matches
0032 — Jacobian callbacks are indexed with global state offsets, so they are wrong for any object that is not first
ID 0032
Class BUG
Severity 3
Status ready
Models Trawl/TrawlDoorBase, Trawl/Vessel, Trawl/CenterWeight, Auv/Vehicle
Found 2026-09-22
Progress 2026-09-22: fixed for Auv/Vehicle only. Trawl/TrawlDoorBase, Trawl/Vessel and Trawl/CenterWeight are untouched and the issue stays open for them.

Evidence

ISimObjectCreator::AddState returns a model-global state offset, but the Jacobian callbacks are handed a local slice. Mixing the two is silently correct for the first stateful SimObject in a model and wrong for every other one.

Framework side, all verified at HEAD of fhsim:

  • AddState returns globalOffset, accumulated across every SimObject in the model: fhsim/src/simobject/SimObjectOrganizer.cpp:470-471, :477. One organizer serves the whole model (fhsim/src/engine/model/ModelAssemblyService.cpp:75) and NewSimObject does not reset the counter (SimObjectOrganizer.cpp:31-39).
  • OdeFcn gets the global vector, un-offset: fhsim/src/engine/model/ModelStructure.cpp:89.
  • OdeJacobian gets a local slice and a local dimension: fhsim/src/engine/model/JacobianAssembler.cpp:468.
  • OutputPortJacobian / InputPortJacobian likewise, state + link.sourceOffset and state + link.consumerOffset: JacobianAssembler.cpp:501-507.

So an AddState return value is right in OdeFcn and wrong in a Jacobian callback. The local index is AddState return value - (this object's smallest AddState return value).

Affected here:

  • src/trawl/CenterWeight.cpp:154-158 (OdeJacobian) and :173-182 (InputPortJacobian)
  • src/trawl/TrawlDoorBase.cpp:204-239 (OdeJacobian)
  • src/trawl/TrawlVessel.cpp:131-151 (OdeJacobian)
  • src/auv/Auv.cpp:255-284 (OdeJacobian)

The same fault exists in fhsim_base (Body/Mass, Math/Integrator, Math/LinearSystem, Math/LowPass) and is filed there as issue base,0021.

Effect

Wrong Jacobian entries for any affected object that is not the first stateful one in a model: derivatives land in the wrong cells, so an implicit solver works from a Jacobian that does not describe the system and either converges slowly or converges somewhere subtly different, with no error reported.

Worse, the destination buffers are sized locally, so a large enough offset writes past the end of the workspace. CenterWeight::InputPortJacobian writes at (m_IStateVel + i) * portSize + i into an nStates x portSize = 6x3 = 18 element buffer; at offset 0 the largest index is 17 and it just fits, but with CenterWeight second in a model its indices run past the end of m_PortJacWorkspace_dF_dInput.

Every affected fixture here has the object under test as the only stateful SimObject, which is why this has never failed. tests/in/CenterWeight/CenterWeight_in.xml holds Env and C; the Environment SimObject registers no states, so C's base offset is 0 and the case passes coincidentally. Putting any stateful object before it in <OBJECTS> breaks it.

Note that this is not the cause of the intermittent failure in issue 0030: that case still diverges when run alone in its own process, so it is not cross-test heap corruption from this bug.

Possible fix

Cache the object's own base offset right after the first AddState and subtract it in every Jacobian callback, leaving OdeFcn and the output-port functions untouched:

m_IStatePos = creator->AddState("Pos", 3); // global, correct for OdeFcn
m_IStateVel = creator->AddState("Vel", 3);
m_IStateBase = m_IStatePos; // this object's global base
// in OdeJacobian / InputPortJacobian / OutputPortJacobian:
const int velLocal = m_IStateVel - m_IStateBase;

Auv is the cheapest one to fix first: it is new, unreleased, and its Jacobian test can be extended to cover the ordering before the older models are touched.

Test that would prove it

For each affected model, add a fixture in which it is second, behind another stateful SimObject, and run with an implicit method so the Jacobians are assembled. Under ASan the current code reports a heap-buffer-overflow in the port-Jacobian path; without ASan the block is silently wrong. A cheaper unit-level check: call the callback directly with a non-zero base offset and assert every write lands inside nStates * portSize.

More generally, a single-object Jacobian fixture cannot detect this class of bug, so the two-object ordering is worth adopting as the default shape for Jacobian fixtures.

Risk

The fix changes the assembled Jacobian for any model where an affected object is not first, so a recorded regression baseline can move. That is a real behaviour change and should be reviewed as one, ideally one model per commit. Against that, the present behaviour includes out-of-bounds writes, so leaving it as it is is not an option.

Progress

**Auv/Vehicle is fixed; the three trawl models are not.**

src/auv/Auv.cpp now caches its own base offset (m_IStateBase = m_IStatePos, right after the last AddState) and every Jacobian callback indexes through Auv::LocalIndex(). OdeFcn, CalcDerivatives and the output port functions keep the raw global indices, which is what they are given. The port Jacobians that increment C deferred on this question are implemented at the same time: InputPortJacobian for ThrustPower and ExternalForce, and OutputPortJacobian for Pos and Battery, the two outputs that are an identity on a state.

tests/in/Auv/Auv_SecondInModel_in.xml puts a second, leading vehicle in front of the one under test, so the Auv's base offset is 14 rather than 0, and drives both of its analytic input ports from that leading vehicle. SimObject.Auv_JacobianBlocksAreCorrectAtANonZeroStateOffset checks the local block and the assembled system against finite differences there. Each claimed entry was confirmed non-vacuous by scaling it by 1.3 and watching the test fail.

Trawl/TrawlDoorBase, Trawl/Vessel and Trawl/CenterWeight are deliberately left alone: fixing them changes the assembled Jacobian of models that exist and can move recorded regression baselines, which is the owner decision this item's Risk section describes. This item stays open for them.

Two things found while doing the Auv half, both worth recording here:

  • The FhSim core does not clear the port-Jacobian workspaces on the direct-link path. JacobianAssembler::AssembleAnalyticalJacobianBlocks hands every direct link the same m_PortJacWorkspace_dPort_dX and m_PortJacWorkspace_dF_dInput (fhsim/src/engine/model/JacobianAssembler.cpp:503-507) and clears neither, while the composed-link path immediately above fills both first (:418-420). Every model in the tree writes only its non-zero entries — CenterWeight.cpp:173-182 even says "caller zero-initialises it" — so one link reads back the previous link's numbers in the cells it skipped, and the assembler multiplies them into its block. Observed here: with two analytic links from the same source object, the ThrustPower column picked up the ExternalForce column and vice versa, giving J[21][0] and J[21][13] the identical wrong value 0.015853 where finite differences said 0.011395 and 0.004458. Auv now clears both matrices itself, which makes it immune, but the defect belongs in the core and affects fhsim_base's Body/Mass and Math/Integrator as they stand.
  • The proving fixture cannot use fhsim_base. The obvious leading object, Body/Mass, is unavailable: fhsim_base is not in this package's dependency closure, which is issue 0028. A second Auv/Vehicle is used instead, which has the side benefit that both ends of the port link are the class under test.