Skip to content

[scenes] Update scenes to be compatible with changes in SOFA regarding IntegrationScheme - #120

Open
epernod wants to merge 8 commits into
masterfrom
fix_compat
Open

epernod wants to merge 8 commits into
masterfrom
fix_compat

Conversation

@epernod

@epernod epernod commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@epernod epernod added pr: fix pr: status to review To notify reviewers to review this pull-request labels Oct 5, 2026

@th-skam th-skam left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's see what regression tests do here.
We may need to do more than just this substitution to adjust to changes from SOFA.

@th-skam th-skam removed the pr: run ci label Oct 6, 2026
@th-skam

th-skam commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

The regression now passes locally with impulseBased=True on the integration scheme. A fix on main sofa is needed for this to work; it's in a PR.

There are some remaining issues which we should revisit:

  • In our non-regression tests, it's only the first and last states that are checked. I regenerated full references with v25.12. Intermediate states do not match that well even though final ones do. The scenes are not largely different but I can confirm that:
    • Our tolerance of 1e-7 is too strict (2 scenes pass with 1e-5).
    • The algorithm has diverged enough since v25.12 for this strict test.
  • Locally I used SofaRegressionProgram in legacy mode. The legacy references store 6 significant digits. SofaRegressionProgram compares the full double-precision states against the rounded ones. More frames leads to higher noise. When the CI is ready to be used with the new JSON format and higher precision we should regenerate them.

@th-skam th-skam self-assigned this Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: fix pr: status to review To notify reviewers to review this pull-request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants