sofa-framework / sofa

Real-time multi-physics simulation with an emphasis on medical simulation.
https://www.sofa-framework.org
GNU Lesser General Public License v2.1
934 stars 312 forks source link

[Core] Modern visitor creation #5126

Open alxbilger opened 1 week ago

alxbilger commented 1 week ago

This PR makes several suggestions, and should not be considered as is for a merge. A choice on the implementations must be made.

Example of the new visitors:

ctx->accept(objectmodel::topDownVisitor
    | [this, &df](core::behavior::BaseForceField* force)
    {
        force->addMBKdx(&mparams, df);
    },

    objectmodel::bottomUpVisitor
    | [this, &df, accumulate](core::BaseMapping* mapping)
    {
        if (accumulate)
        {
            mapping->applyJT(&mparams, df, df);
            if( mparams.kFactor() != 0_sreal )
            {
                mapping->applyDJT(&mparams, df, df);
            }
        }
    });

Example of the old visitors:

executeVisitor( makeMechanicalVisitor(&mparams,
    TopDownMassCallable([this, &result](simulation::Node* /*node*/, core::behavior::BaseMass* mass)
    {
        if( mass->m_separateGravity.getValue() )
        {
            mass->addGravityToV(&mparams, result);
        }
        return Visitor::RESULT_CONTINUE;
    })
) );

Pros:

Cons:

TODO with the new visitors:

The question is: which implementation of the visitors do we keep?

Future work: Implement this kind of "visitor" for the mapping graph


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

alxbilger commented 1 week ago

The unit tests fail because lambdas on BaseForceField are not called on BaseInteractionForceField, which is the case in old visitors.

fredroy commented 1 week ago

[ci-build][with-all-tests]

hugtalbot commented 6 days ago

@sofa-framework/reviewers your opinion is needed.

While working on the visitor definition, Alex proposes two alternative methods: the creation of classical SOFA visitors or a new implementation of the original visitor design pattern. It is a known pattern, slightly more verbous but the implementation follows modern C++ format (accept combined with lambda). A second version of these modern visitors is presented (see MechanicalOperations.cpp) with a function e.g. makeMechanicalVisitor. This might lead to code duplication. Objective is to improve the visitor readability at first sight. This work is done with the objective to apply visitors on a "mapping-based graph".

We thought @JeremieA you could have an interesting point of view about this.