Tune collocated integral-error saturation to tendon scale - #135
Open
mstoelzle wants to merge 2 commits into
Open
Tune collocated integral-error saturation to tendon scale#135mstoelzle wants to merge 2 commits into
mstoelzle wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Section V.d collocated gain-optimization script to use a physically interpretable integral-error saturation scale (tendon-length error in meters) instead of a fixed, opaque gamma constant, and documents that the currently committed Section V.d artifacts are now stale.
Changes:
- Replace the collocated optimizer’s fixed
gamma=10withgamma = 1 / e_satusing a defaulte_sat = 0.01 m. - Add
--integral-error-saturation-scaleCLI option with validation for finite, strictly positive values. - Document the derivation/scale selection and mark committed Section V.d MAT/plots as stale in the case README.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| paper_results/secVd_control_gain_optimization/README.md | Adds a prominent stale-artifacts warning and documents the tendon-scale saturation rationale and usage. |
| paper_results/secVd_control_gain_optimization/code/control_gain_optimization_with_collocated.py | Introduces a configurable tendon-length saturation scale, validates it, and computes gamma as its reciprocal for PID tanh saturation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
paper_results/secVd_control_gain_optimization/code/control_gain_optimization_with_collocated.py:266
- After validating
--integral-error-saturation-scaleitself,tendon_error_gamma = 1.0 / tendon_error_saturation_scalecan still overflow to a non-finite value for extremely small but positive (subnormal) scales. That failure would currently surface later as a PIDControl "Gamma must be finite" error, which is less actionable for CLI users. Consider validating that the computedtendon_error_gammais finite and raising a ValueError that points back to the CLI flag.
tendon_error_saturation_scale = ARGS.integral_error_saturation_scale # [m]
tendon_error_gamma = 1.0 / tendon_error_saturation_scale # [1/m]
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.
Summary
gamma=10with a physically interpretable default tendon-error scale ofe_sat = 10 mm, i.e.gamma = 100 1/m--integral-error-saturation-scale(meters) and reject non-finite or non-positive valuesgammavalues universally inPIDControlbefore positivity/SPD validationHow we arrived here
PR #133 corrected the built-in saturation from the former
tanh(gamma * e)expression to the unit-preservingThe collocated control-gain optimizer integrates only tendon-length errors, so every controlled coordinate has units of meters and a scalar
gammahas units1/m. The optimizer had retainedgamma=10, which now meanse_sat=100 mm: the full segment length and far outside the observed error regime. It retained 99.3% of the largest initial error, making saturation effectively inactive.I transformed every saved configuration in the six committed initial and optimized collocated trajectories into tendon coordinates and compared them with the tendon-length setpoints:
Apart from the exceptional 14.51 mm reference step, all observed initial/transient absolute errors across the saved trajectories remain below 3.85 mm.
A 10 mm saturation scale is therefore a deliberate boundary between the large reference step and the ordinary closed-loop error regime:
sat(e)=8.96 mm, reducing integral accumulation by 38.25%This limits windup during the large step without materially reshaping the ordinary transient and regulation errors.
Result status and regeneration order
Important
All currently committed Section V.d MAT files and derived plots must now be considered stale. The collocated results were generated with the former non-unit-preserving saturation and the former gamma setting. This PR intentionally does not replace any result artifacts.
The complete Section V.d results should be regenerated only after:
A one-iteration temporary smoke run completed the simulation/save path, but it reproduced the non-finite optimized gains tracked in #129. Its setpoint rollout also warned that it had not reached steady state. Those outputs were written only under
/tmpand are not included here.Validation
ruff checkandruff format --checkon the collocated optimizerpython -m py_compileon the collocated optimizerpytest -q tests/control/test_pid_control.py: 41 passed@Michele-Martini, could you please review the physical scale selection and the intended regeneration sequencing?