[SofaBaseTopology] Pr change api numerical integration

[hdeling]

ADD : use a string instead of an enum to specify an integration method as it easier for user to add new ones. This slightly changes the API of numerical integration. ADD : add the possibility to add a function to create on the fly the required integration points (when an analytical formula is available). UPDATE : modified the name of the integration methods for edges, triangles, tetrahedra and hexahedra. UPDATE : use of numerical integration in Flexible FIX : bug corrected when using of BezierShapeFunction with plugin HighOrderMeshes

---

This PR:

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

Reviewers will merge only if all these checks are true.

[marques-bruno]

Hi @hdeling, Could you give us more precisions regarding the need to change from enums to strings for the integration methods?

If the goal is to simplify the conversion from the string (passed in XML / Python scenes) to the C++ enums, wouldn't it be best to take advangate of the OptionsGroup datatype already present in SOFA?

[hdeling]

Could you give us more precisions regarding the need to change from enums to strings for the integration methods?

Two reasons motivated the use of strings :

1. Right now when specifying the type of numerical integration method in python or xml scripts the user had to use for instance numericalIntegrationMethod="3" which is not descriptive at all. It is preferable to use numericalIntegrationMethod="Tetrahedron Gauss" for the maintenance of the code. Using OptionsGroup could solve this issue I believe.

2. Inheritance. In the plugin "SofaHighOrder" I specify new types of numerical integration methods on triangles. While the APi allows to add a new method, the enum class cannot be extended ( I can use a number like 4 but this creates warnings and is not satisfactory). Therefore using a string class is more versatile.

If the goal is to simplify the conversion from the string (passed in XML / Python scenes) to the C++ enums, wouldn't it be best to take advangate of the OptionsGroup datatype already present in SOFA?

This would not solve the inheritance problem as the type of numerical integration methods can be exhaustively listed.

[VannesteFelix]

Hi @hdeling,

thanks for the explanations. Is it ok for you @marques-bruno ?

[guparan]

Hi guys, what about replacing the enum by a class containing static int members? see https://stackoverflow.com/a/644639

[hdeling]

Hi,

static const int solves the inheritance of enum class, but cannot really be used for the purpose of typing a categorical variable.

For instance we can have this :

class NumericalIntegrationMethod { public : static const int methodOne=1;};

class NewNumericalIntegrationMethod : public NumericalIntegrationMethod  { public : static const int methodTwo=2; };

but the integration method has to be described as :

int method=NumericalIntegrationMethod::methodOne; or int method=NewNumericalIntegrationMethod ::methodTwo;

but not as : NumericalIntegrationMethod method; (which would be done with enum).

Anyway, the use of string instead of enums is also motivated with the issue of having descriptive labels in python or xml files.

See my previous comments below :

Right now when specifying the type of numerical integration method in python or xml scripts the user had to use for instance numericalIntegrationMethod="3" which is not descriptive at all. It is preferable to use numericalIntegrationMethod="Tetrahedron Gauss" for the maintenance of the code

Hervé

Le 13/03/2019 à 10:16, Guillaume Paran a écrit :

Hi guys, what about replacing the enum by a class containing static int members? see https://stackoverflow.com/a/644639

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sofa-framework/sofa/pull/936#issuecomment-472340591, or mute the thread https://github.com/notifications/unsubscribe-auth/ASgmq8xcftMg8X0iZv9ijlhfWBJGHFwCks5vWMHsgaJpZM4a_sTV.

[marques-bruno]

@hdeling, @guparan, I think the idea would be to combine both the use of OptionsGroup with the wrapping of enums as described in the link Guillaume posted. Wouldn't something like this POC address your problem without manipulating strings from the C++?:

#include <sofa/helper/OptionsGroup.h>
#include <sofa/core/objectmodel/Data.h>

struct NumericalIntegrationMethod : public sofa::core::objectmodel::BaseObject
{
    struct Method {
        enum
        {
            One = 1,
            Two,
            Last
        };
    };


    Data<sofa::helper::OptionsGroup> d_method;


    NumericalIntegrationMethod()
        : d_method(initData(&d_method, "method",
                                      "sets the chosen numerical integration method"))
    {
        sofa::helper::OptionsGroup* o = d_method.beginEdit();
        o->setNames(2, "One", "Two");
        o->setSelectedItem(Method::One);
        d_method.endEdit();
    }

    unsigned method() { return d_method->getSelectedID(); }
    void setMethod(unsigned m) { d_method->setSelectedItem(m); }
};

struct NewNumericalIntegrationMethod : public NumericalIntegrationMethod
{
    struct Method : public NumericalIntegrationMethod::Method{
        enum
        {
            Three = NumericalIntegrationMethod::Method::Last,
            Last
        };
    };

    NewNumericalIntegrationMethod()
    {
        sofa::helper::OptionsGroup* o = d_method.beginEdit();
        o->setNbItems(NumericalIntegrationMethod::Method::Last + 1);
        o->setItemName(3, "Three");
        o->setSelectedItem(NewNumericalIntegrationMethod::Method::Three);
        d_method.endEdit();
    }

};

/*
/// From C++
NewNumericalIntegrationMethod method;
method->setMethod(NewNumericalIntegrationMethod::Method::Two);

# in Pyhton || XML:
 <NewNumericalIntegrationMethod method="Two" />
*/

(I coded this quickly, didn't try nor compile it, but this should work I believe)

[guparan]

@hdeling Did you have some time to read Bruno's proposal above?

[hugtalbot]

Let's rediscuss about @marques-bruno proposal next Wednesday! Then I can take care of it.

[hugtalbot]

You weren't there this morning @marques-bruno but if your proposal is both :

• specifying in a descriptive way the type of numerical integration method
• the inheritance issue

and I think it does. We should go for it. I did not know the OptionsGroup class. If not, we keep and merge Hervé's work.

Hugo

[hugtalbot]

@jnbrunet have you taken a look at this PR?

Latest proposal of Damien is to have:

• Unique identifier for topologies (as Datafield)
• Create QuadratureMethod component (datafield method, datafield topology → datafield function pointer with integration points)