fix(mechanism): make set_constraints change the dimensions the solver maintains - #58
Merged
Merged
Conversation
… maintains Link lengths live in Link._cached_distances and in the group solver's Dimensions, both captured when the mechanism is built. set_constraints only moved the driver's output joint, so the next step() re-solved every joint against the original lengths and snapped the crank tip back to its original radius; get_constraints() (position-based for binary links) then read the old values too. Optimizing a Mechanism was a silent no-op. - Link.set_distance(j1, j2, d): write the constraint (both orderings), positions untouched, re-solved on the next step. - Mechanism.set_constraints: write driver radii and binary link lengths through set_distance, reposition the crank tip at its current angle, rebuild the Assur decomposition so the group solver sees the new Dimensions. Skips the same links get_constraints skips. - Link.length reads get_distance() (the cache when built), consistent with DriverLink.radius, so get/set_constraints round-trip before any step. Tests cover the round trip, both solver paths maintaining the new lengths over 20 steps, and dict serialization after a dt=0 step.
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
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.
Found while wiring the editor's optimization endpoint onto
Mechanism(pylinkage-editor).Bug
Mechanism.set_constraints()did not change the mechanism. Link lengths live inLink._cached_distancesand in the Assur-group solver'sDimensions, both captured at construction.set_constraintsonly moved the driver's output joint, so the nextstep()re-solved every joint against the original lengths — and put the crank tip back at its original radius, sinceDriverLink.radiusreads the cache.get_constraints()(position-based for binary links) then read the old values too.Reproduce on 1.2.1:
Consequence: every optimizer run on a
Mechanism(documented as "used for optimization" since 0.8.0) evaluated the same geometry for every particle.Fix
Link.set_distance(j1, j2, d)— writes the constraint (both orderings); positions are untouched and re-solved at the next step. Rejects non-positive distances.Mechanism.set_constraints— writes driver radii and binary link lengths throughset_distance, repositions the crank tip at its current angle as before, and rebuilds the Assur decomposition so the group solver'sDimensionsfollow (0.02 ms on a four-bar vs ~10 ms per cycle simulation). Skips exactly the linksget_constraintsskips, so the two stay aligned.Link.lengthreadsget_distance()(the maintained constraint once built), consistent withDriverLink.radius, soget/set_constraintsround-trip before any step.Scope unchanged: only binary links and driver radii form the constraint vector; n-ary links are still left out, as before.
Tests
TestGetSetConstraints: round trip before stepping; both solver paths (group_solverTrue/False) maintain the new lengths over 20 steps;to_dict/from_dictafter adt=0step keeps them.TestLinkDistances:set_distancesymmetry, positions untouched, validation. Full suite 2614 passed; ruff, mypy clean.