Skip to content

[CORE] General discussion regarding QUADRATURES #13085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AlejandroCornejo opened this issue Feb 3, 2025 · 16 comments · May be fixed by #13443 or #13444
Open

[CORE] General discussion regarding QUADRATURES #13085

AlejandroCornejo opened this issue Feb 3, 2025 · 16 comments · May be fixed by #13443 or #13444

Comments

@AlejandroCornejo
Copy link
Member

AlejandroCornejo commented Feb 3, 2025

Description:
While implementing a new element (#12897), I needed to add Lobatto quadratures.

Currently, Lobatto quadratures exist in Kratos for some geometries, but not all, and they’re not generally usable for arbitrary orders or integrations. To address this, I added order-one Lobatto quadratures (LOBATTO_1) for the classical geometries:

  • Linear triangle
  • Quadrilateral
  • Tetrahedron
  • Hexahedron

Open Issues

Purpose of GI_GAUSS_EXTENDED:
The GI_GAUSS_EXTENDED quadratures appear inconsistent. Depending on the geometry, they either perform collocation or behave identically to standard Gauss quadratures.

  • What is their intended general purpose?
  • Do we actually need them, given these inconsistencies?

Handling Missing Quadrature Implementations:
As it stands, all entries of the quadrature enumeration must be filled, even for cases where specific implementations are unavailable. This leads to duplicated definitions like using Gauss quadratures for GI_EXTENDED.

  • Should we adjust this requirement or provide a fallback mechanism?

Looking for feedback and suggestions on these points to improve the quadrature handling in Kratos.

@loumalouomega @rubenzorrilla @RiccardoRossi

@loumalouomega
Copy link
Member

loumalouomega commented Feb 3, 2025

GI_GAUSS_EXTENDED was originally added by me, and the poupouse was to be used by the solid-shell. My fault, because after that we have been using it as catch-all (cajón de sastre). My suggestion is to define proper names and keep GI_GAUSS_EXTENDED with current behaviour. I think the problem is that this overloads the size of the geometry class make it more inefficient, so maybe we should rethink completely the integration strategies .

@matekelemen
Copy link
Contributor

matekelemen commented Feb 3, 2025

Since it's labelled a general discussion, I'll go ahead and aim a suggestion at @KratosMultiphysics/nurbs-breps and @KratosMultiphysics/mpm.

In my opinion, both the IGA and MPM apps struggle to align their abstractions with Kratos' layout, partially due to the relatively rigid quadrature system.

I'm not exactly sure why, but both applications treat integration points as elements even though they'd have more conventional options. I'm not too familiar with either field, but I'd imagine that:

  • using the background elements with a dynamic quadrature rule would fit MPM better. A quadrature rule could be constructed at each solution step, defaulting to the current implementation, but allowing the construction of a reduced integration in case of too many particles in a single background element.
  • using knotspans as elements would be much more intuitive for IGA in my mind. Of course its integration rule has to depend on its polynomial order and trimming.

In any case, I think it would be neat if each element/geometry instance had the possibility to own its own integration object (kinda similar to Properties - in most cases all elements share the same Properties but if necessary, they could share subsets or just have their own).

@rubenzorrilla
Copy link
Member

GI_GAUSS_EXTENDED was originally added by me, and the poupouse was to be used by the solid-shell. My fault, because after that we have been using it as cath-all (cajón de sastre). My suggestion is to define proper names and keep GI_GAUSS_EXTENDED with current behaviour.

After a brief discussion yesterday, @RiccardoRossi @AlejandroCornejo and myself reached consensus on removing them if are of course not being used. If they are being used, we'd implement them consistently for all the geometries, not necessarily for order 1 to 5.

I think the problem is that this overloads the size of the geometry class make it more inefficient, so maybe we should rethink completely the integration strategies .

This is what we also spoke about. The point is that, as you already know, everything must happen at compile time, something that turns into an extremely tight design constrain.

@rubenzorrilla
Copy link
Member

* using the background elements with a dynamic quadrature rule would fit MPM better. A quadrature rule could be constructed at each solution step, defaulting to the current implementation, but allowing the construction of a reduced integration in case of too many particles in a single background element.

This I don't think is possible. Also I don't think it is a good idea. Note that integration quadratures cannot be dynamic for the sake of efficiency. Right now, everything happens at compile time, so calling the integration points has no cost.

* using knotspans as elements would be much more intuitive for IGA in my mind. Of course its integration rule has to depend on its polynomial order and trimming.

Ideally yes, but note that our IGA is not "classical" IGA but IBRA, hence, we will always have the trimming. Having said this, we will always have to deal with the trimming, which necessarily needs to happen at runtime and hence goes against the need of setting up the integration points at compile time.

@rickyaristio
Copy link
Contributor

rickyaristio commented Feb 4, 2025

Small comment regarding IGA/IBRA: We plan to have the possibility of creating one layer abstraction by adding KnotSpanGeometry, which is going to use knotspans as elements. The idea is to collect the list of integration points and assign it as one geometry so that we have one element/condition per set of integration points. Note that this will not depreciate the idea of QuadraturePointGeometry as this still might be used in some cases, such as point on a surface/curve or the MPM application. In a couple of weeks, we will prepare the PR.

CC: @manuelmessmer

@rubenzorrilla
Copy link
Member

rubenzorrilla commented Feb 4, 2025

Having said all this, let me try to put in short the bullet points that we need to address (some of them are of course wishes):

  • We need the quadratures "to happen" at compile time
  • We need to fix the catch-all (cajón desastre) issue with the GI_EXTENDED quadratures
  • When adding a quadrature for one geometry, we would like to avoid the need to define it for all the other geometries.
  • If the one above is not possible, would be nice to have a mechanism to throw a runtime no-implementation error (rather than putting anything just for the sake of having it defined as we did so far).

Let me note that some of these complexities and inconveniences come from the fact that we are relying on enum(s) wrap the integration methods. This is the unique thing that was available in ancient days to accomplish with the compile time requirement, but, maybe, with modern c++ we can do something else (pinging @roigcarlo for an imaginative solution).

@loumalouomega
Copy link
Member

Mentioning @pablobecker as expert in dynamic allocated quadratures

@matekelemen
Copy link
Contributor

matekelemen commented Feb 4, 2025

We need the quadratures "to happen" at compile time

It's unlikely that I'll personally touch kratos quadrature in the near future so I'm not requesting any changes here, but I'll just share my unsolicited thoughts on it:

I agree that storing shape function values at predetermined locations instead of evaluating them on the fly works very well for low order elements/geometries. That said, making this a requirement for every geometry and forbidding other approaches locks us out of whole families of methods (FCM comes to mind first, but I really mean any approach that relies on tweaking its shape functions or quadrature dynamically). This is quite a shame because kratos is primarily a research code and most other parts of our infrastructure would be able to support such methods.

Note that integration quadratures cannot be dynamic for the sake of efficiency. Right now, everything happens at compile time, so calling the integration points has no cost.

The only cost we're sparing is the calculation of shape functions at each integration point. We still have to fetch cached shape function values from memory, which is absolutely not a negligible cost.

@loumalouomega
Copy link
Member

We can think alternative approaches, but I think I will discuss with @pooyan-dadvand first, because I think my idea woul have a performance penalty.

@rubenzorrilla rubenzorrilla moved this to 👀 Next meeting TODO in Technical Commiittee Feb 5, 2025
@RiccardoRossi
Copy link
Member

we had a first round of discussion on this one, taking also into account the feedback of @matekelemen

as of today IT IS possible to store in a geometry a "flexible" GeometryData (chancing the pointer at construction time). This needs to be better documented but should take into account the use case of @matekelemen

per regards the EXTENSION_GAUSS our impression is that they should be eventually removed to prevent usage as catch all

this is very preliminary and we will iterate it further, nevertheless our current impression is that the integration rules should have:

  • GAUSS, order
  • LOBATTO, order
  • COLLOCATION, order

anyhowthis would be a breaking change so it is just a working idea.

please note however that EXTENSION_:GAUSS with different meaning should NOT be allowed

@RiccardoRossi RiccardoRossi moved this from 👀 Next meeting TODO to 🏗 In progress in Technical Commiittee Feb 11, 2025
@loumalouomega
Copy link
Member

we had a first round of discussion on this one, taking also into account the feedback of @matekelemen

as of today IT IS possible to store in a geometry a "flexible" GeometryData (chancing the pointer at construction time). This needs to be better documented but should take into account the use case of @matekelemen

per regards the EXTENSION_GAUSS our impression is that they should be eventually removed to prevent usage as catch all

this is very preliminary and we will iterate it further, nevertheless our current impression is that the integration rules should have:

* GAUSS, order

* LOBATTO, order

* COLLOCATION, order

anyhowthis would be a breaking change so it is just a working idea.

please note however that EXTENSION_:GAUSS with different meaning should NOT be allowed

COLLOCATION is not known at builiding time, am I right?

@RiccardoRossi
Copy link
Member

well, you could have a collocation known at building time, for example dividing a triangle in 4, 16 and so on

@rubenzorrilla rubenzorrilla moved this from 🏗 In progress to 👀 Next meeting TODO in Technical Commiittee Feb 27, 2025
@AlejandroCornejo
Copy link
Member Author

So, in practical terms, did we reach any consensus regarding this issue?@KratosMultiphysics/technical-committee

@rubenzorrilla
Copy link
Member

Even though I note @loumalouomega 's initial intentions, I'd start by removing the extended Gauss quadratures as these are only correctly implemented for a few geometries and, as far as I know, these are not being used.

@RiccardoRossi
Copy link
Member

We have been further discussing this in @KratosMultiphysics/technical-committee

as a short term fix we shall change this list:

    enum class IntegrationMethod {
        GI_GAUSS_1,
        GI_GAUSS_2,
        GI_GAUSS_3,
        GI_GAUSS_4,
        GI_GAUSS_5,
        GI_EXTENDED_GAUSS_1,
        GI_EXTENDED_GAUSS_2,
        GI_EXTENDED_GAUSS_3,
        GI_EXTENDED_GAUSS_4,
        GI_EXTENDED_GAUSS_5,
        GI_LOBATTO_1,
        NumberOfIntegrationMethods // Note that this entry needs to be always the last to be used as integration methods counter
    };

to

    enum class IntegrationMethod {
        GI_GAUSS_1,
        GI_GAUSS_2,
        GI_GAUSS_3,
        GI_GAUSS_4,
        GI_GAUSS_5,
        GI_LOBATTO_1,
        GI_EXTENDED_GAUSS_1,
        GI_EXTENDED_GAUSS_2,
        GI_EXTENDED_GAUSS_3,
        GI_EXTENDED_GAUSS_4,
        GI_EXTENDED_GAUSS_5,
        NumberOfIntegrationMethods // Note that this entry needs to be always the last to be used as integration methods counter
    };

This implies the requirement of implementing the first order lobatto quadrature for all available geometries.

In the mid term, the EXTENDED* quadratures should be fixed to be consisten across geometries, which would imply ideally naming them in a more self descriptive way.

@rubenzorrilla will suprvise this syncronizing with @loumalouomega and @AlejandroCornejo

@RiccardoRossi RiccardoRossi moved this from 👀 Next meeting TODO to 🏗 In progress in Technical Commiittee Mar 12, 2025
@loumalouomega
Copy link
Member

We have been further discussing this in @KratosMultiphysics/technical-committee

as a short term fix we shall change this list:

enum class IntegrationMethod {
    GI_GAUSS_1,
    GI_GAUSS_2,
    GI_GAUSS_3,
    GI_GAUSS_4,
    GI_GAUSS_5,
    GI_EXTENDED_GAUSS_1,
    GI_EXTENDED_GAUSS_2,
    GI_EXTENDED_GAUSS_3,
    GI_EXTENDED_GAUSS_4,
    GI_EXTENDED_GAUSS_5,
    GI_LOBATTO_1,
    NumberOfIntegrationMethods // Note that this entry needs to be always the last to be used as integration methods counter
};

to

enum class IntegrationMethod {
    GI_GAUSS_1,
    GI_GAUSS_2,
    GI_GAUSS_3,
    GI_GAUSS_4,
    GI_GAUSS_5,
    GI_LOBATTO_1,
    GI_EXTENDED_GAUSS_1,
    GI_EXTENDED_GAUSS_2,
    GI_EXTENDED_GAUSS_3,
    GI_EXTENDED_GAUSS_4,
    GI_EXTENDED_GAUSS_5,
    NumberOfIntegrationMethods // Note that this entry needs to be always the last to be used as integration methods counter
};

This implies the requirement of implementing the first order lobatto quadrature for all available geometries.

In the mid term, the EXTENDED* quadratures should be fixed to be consisten across geometries, which would imply ideally naming them in a more self descriptive way.

@rubenzorrilla will suprvise this syncronizing with @loumalouomega and @AlejandroCornejo

I am OK, the place where I use my extended gauss points is quite hardcorded, so it can be a little bit more hardcoded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment