Repository navigation
fix(hermite): keep the cached committed force across snapshot and restore - #46
Merged
Merged
Conversation
…tore A restore followed by a step must be bit-identical to taking that step from the saved state (CableDyn_RestoreState, OpenFAST correction iterations). Restore discarded the committed force cached from the previous step and evaluated it afresh. On a finite-EI cable with seabed friction the two differ in the last bits: the commit's return mapping moves a sliding node's anchor to q - c*dxy, and the spring force from q minus that anchor reproduces the capped force only to rounding. Whether it showed depended on the trajectory (the controlled-cable snapshot test failed on the Linux Debug runner only). Snapshot now keeps the cache with the state, and Restore brings it back unless a setter invalidated the cache since the snapshot (fc_clears counts the invalidations, including the OpenFAST checkpoint reload). The control test asserts the restored cache again instead of clearing the reference's, and the cache comment states what holds. Runs without restores do not change. Signed-off-by: Jae Hoon Seo <jaehoon.seo@inha.ac.kr>
Owner
Author
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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
First item of the correctness backlog before 0.3.0.
The problem. The C API manual promises that a restore followed by a step is bit-identical to taking that step from the saved state (
CableDyn_SaveState/RestoreState, used for CFD outer correctors). OpenFAST correction iterations rely on the same Snapshot/Restore pair. On a finite-EI cable with seabed friction, the promise could fail in the last bits:CD_HermiteCable_Dyn_Restorediscarded that cache, so the restored model evaluated F_n afresh.q - c*dxy. The spring force formed fromqminus that anchor reproduces the capped force only to rounding. So the fresh F_n differs from the cached one in the last bits.What changes:
fc_R, its key and its validity.fc_clears, is advanced at every invalidation: all 16 setter sites and the OpenFAST checkpoint reload. A restored model therefore takes the same path as one that was never rolled back.Runs without restores take the same path as before.
Verification
assemble_contactforms the capped force fromq - anchor_old, whilecommit_friction_anchorswritesanchor_new = q - (cap/(k*dlen))*dxy.pre-commit run --all-filesand the Sphinx-Wbuild pass.-O0 -fcheck=all).Checklist
pre-commit run --all-filespassesctest --test-dir build --output-on-failurepassespython -m pytest tests python/testspasses (if Python or validation records changed)[Unreleased](heading added if missing) for user-visible changesdoc/updated for changed inputs, options, or outputs (no change: the documented behaviour now holds)feat:,fix:,docs:, ...)git commit -s) under theDeveloper Certificate of Origin