Rebuild flattened-model schemas on FrameSystem.UnmarshalJSON#5961
Draft
NickPPC wants to merge 1 commit into
Draft
Rebuild flattened-model schemas on FrameSystem.UnmarshalJSON#5961NickPPC wants to merge 1 commit into
NickPPC wants to merge 1 commit into
Conversation
A FrameSystem round-tripped through JSON lost the componentSchemas, flattenedModels, and mimicFrames lookup tables. These are populated at construction time by flattenModelIntoFS but are not part of the on-disk format, so after UnmarshalJSON any Transform that reached a flattened per-joint frame (e.g. "arm1:joint_1") would hit resolveFrameInputs, find no schema, and fail with NewIncorrectDoFError. Every serialized PlanRequest with a multi-DoF arm was affected — validatePlanRequest would panic before planning could start. Rebuild the maps from the restored SimpleModel's inputSchema / mimicMappings, guarded on the namespaced sub-frames actually being present in fs.frames so models added directly via AddFrame (which are not flattened) are left alone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
viambot
added
the
safe to test
Contributor
Availability
Quality
Performance
The above data was generated by running scenes defined in the
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A
FrameSystemround-tripped through JSON lost the derived lookup tables (componentSchemas,flattenedModels,mimicFrames) that map a component's flat input vector onto its flattened per-joint sub-frames. AfterUnmarshalJSON, anyTransformthat reached a namespaced sub-frame (e.g.arm1:joint_1) would fall throughresolveFrameInputs, find no schema, and panic withNewIncorrectDoFError(0, 1)— so every serializedPlanRequestwith a multi-DoF arm panicked insidevalidatePlanRequestbefore planning could start.Repro:
go run motionplan/armplanning/cmd-plan/cmd-plan.go -v <any PlanRequest JSON with a 6-DoF arm>→panic: array length does not match frame DoF, expected 1 but got 0.Changes
UnmarshalJSONnow initializesmimicFrames(was left nil) and calls a newrebuildFlattenedSchemas()pass. The helper walks restored frames, unwraps*namedFramewrappers, and for each*SimpleModelwith DoF > 0 repopulatesflattenedModels/componentSchemas/mimicFramesfrom the inner model's already-restoredinputSchemaandmimicMappings. Guarded on the namespaced sub-frames actually being present infs.frames, so models attached directly viaAddFrame(whichflattenModelIntoFSnever touched) are left alone — this preservesTestSerialization's behavior whereFrameNames()would otherwise try to subtract sub-frames that don't exist.Testing
TestFlattenedFrameSystemJSONRoundTripinreferenceframe/frame_system_flatten_test.go: builds aNewFrameSystemwith a real flattened arm + child frame, round-trips through JSON, then drivesTransform(camera → world) andComputePoseswith component-level inputs. Both paths panicked before the fix and pass after. Also compares the post-unmarshal transform against the original FS to catch silent divergence../referenceframe/...and./motionplan/...suites pass locally (two referenceframe tests —TestUR20URDFWithMeshes,TestMeshGeometrySerialization— failed on artifact-fetch auth and are unrelated).PlanMotionnow runs through, surfacing the actual content-level IK/collision result for the input file.Claude Code Prompts Used