Skip to content

[LinearSystem] Check if state is mapped to contribute to the global vector#6081

Open
alxbilger wants to merge 1 commit intosofa-framework:masterfrom
alxbilger:mappingglobalvector
Open

[LinearSystem] Check if state is mapped to contribute to the global vector#6081
alxbilger wants to merge 1 commit intosofa-framework:masterfrom
alxbilger:mappingglobalvector

Conversation

@alxbilger
Copy link
Copy Markdown
Contributor

@alxbilger alxbilger commented Apr 15, 2026

Consider this scene:

def createScene(root):
    root.addObject("DefaultAnimationLoop")

    root.addObject("EulerExplicitSolver", symplectic=False, printLog=True)
    # root.addObject("EulerImplicitSolver", printLog=True)
    root.addObject("SparseLDLSolver", template="CompressedRowSparseMatrix")

    with root.addChild("main_particle") as main_particle:
        initial_position = [0.0, 0.0]
        initial_velocity = [1.0, 1.0]
        the_particle = main_particle.addObject("MechanicalObject", template="Vec2", name="particle",
            position=[initial_position], velocity=[initial_velocity])

    with root.addChild("first_level_mapping") as first_level_mapping:
        mapped_particle_1 = first_level_mapping.addObject("MechanicalObject", template="Vec2", name="mapped_particle")

    with root.addChild("second_level_mapping") as second_level_mapping:
        mapped_particle_2 = second_level_mapping.addObject("MechanicalObject", template="Vec2", name="mapped_particle")
        second_level_mapping.addObject("UniformMass", template="Vec2", name="mass", vertexMass=1.0)

    # This mapping acts on the second level, and its applyJT should be executed first in the mapping graph order, but
    # due to its location in the scene graph, it is executed after the other mapping.
    # However, because of the bug in the visitors of this PR, the contribution from the mapped state is copied into the global vector anyway. Two bugs mutually cancelling each other out.
    root.addObject("IdentityMapping", input=mapped_particle_1.linkpath, output=mapped_particle_2.linkpath)

    with root.addChild("mapping_2") as mapping_2:
        mapping_2.addObject("IdentityMapping", input=the_particle.linkpath, output=mapped_particle_1.linkpath)

    root.addObject("VisualPointCloud", position=the_particle.position.linkpath, sphereRadius=0.05, drawMode="Sphere", color="navy")
    root.addObject("TrailRenderer", template="Vec2", position=the_particle.position.linkpath)

    return root
wrong_mapping_0000

This scene does not pass the scene check. The reason is that it does not follow the logic of the visitors, so an error is emitted at the beginning of the simulation (but not if run directly in Python as SceneCheckers don't run). This is expected, and it's not a wrong error. Let's try to make this simulation working even if it does not pass the SceneChecker.

The Issue:
Currently, the simulation produces correct results (a parabola) despite the SceneChecker error. This is due to two bugs inadvertently cancelling each other out:

Execution Order Bug: Due to its position in the scene graph, a specific mapping executes after others, even though its applyJT should execute first.
Data Propagation Bug: The visitors in this PR do not correctly verify if they are operating on "main states." They incorrectly treat mapped_particle as a main state because no mapping is defined within that specific node. Consequently, the contribution from the mapped state is copied into the global vector anyway. This PR addresses this bug.

The Fix:
I have updated the visitors to use the existing MappingGraph. Instead of relying on the presence (or absence) of a mapping within a local node, the visitor now correctly identifies if a state is part of a mapping via the global graph.

Note:
Now the scene fails. The trajectory is no longer a parabola, but a straight line. It is expected because one bug remains.

[with-all-tests]


By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).


Reviewers will merge this pull-request only if

  • it builds with SUCCESS for all platforms on the CI.
  • it does not generate new warnings.
  • it does not generate new unit test failures.
  • it does not generate new scene test failures.
  • it does not break API compatibility.
  • it is more than 1 week old (or has fast-merge label).

@alxbilger alxbilger added pr: fix Fix a bug pr: status to review To notify reviewers to review this pull-request labels Apr 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: fix Fix a bug 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.

1 participant