KratosMultiphysics / Kratos

Kratos Multiphysics (A.K.A Kratos) is a framework for building parallel multi-disciplinary simulation software. Modularity, extensibility and HPC are the main objectives. Kratos has BSD license and is written in C++ with extensive Python interface.
https://kratosmultiphysics.github.io/Kratos/
Other
1.03k stars 245 forks source link

CL Parameters : Save pointer to element instead of pointer to geometry #9775

Open pablobecker opened 2 years ago

pablobecker commented 2 years ago

Description

I have to access the flags stored in the element.

Unlike the variables (GetValue,SetValue) which are stored in the geometry, the flags are stored at geometricalObject level. I understand this is done like this so that flags like ACTIVE, etc, can be set with differnt values between the different elements that share the geometry.

So what I'd like to have is a pointer to the element and delete the one to the geoemtry. CLs are very specific to the element they are associated to, so it would not add confussion if we set the pointer to this specific element. Access to the geometry will still be available using mpElem->GetGeometry().

ddiezrod commented 2 years ago

It makes sense to me as I think there is no other option to send this info to the CL, and also, as Pablo said, elements are CLs are closely related. What do you think? @KratosMultiphysics/technical-committee @KratosMultiphysics/implementation-committee

loumalouomega commented 2 years ago

I thought it was already the element...

loumalouomega commented 2 years ago

I thought it was already the element...

Nope, but the name is mpElementGeometry

loumalouomega commented 2 years ago

Ok from my side

ddiezrod commented 2 years ago

We were discussing this in @KratosMultiphysics/implementation-committee We agree on adding a new GetElement method in the CL. In order to minimize the changes we can modify the GetElementGeometry like this;

GetElementGeometry()
{
    return GetElement().GetGeometry();
}

but still, we will need to do changes in the elements to call the new SetElement method. Also, we agreed that the element provided to the CL should be const.

rubenzorrilla commented 2 years ago

Although I like the idea, I consider this is a major change that needs to be deeply discussed. So here are my two cents.

About removing the pointer to the geometry I'm concerned about the fact that this will obligue to always define an element to use the constitutive law. This means that constitutive laws will no longer be compatible with conditions, something that it is right now technically possible. We need to be sure that this is the way we want to go.

Besides, this changes needs to be retrocompatible for a very long period of time. The unique thing that comes to my mind is to have two pointers, one for geometry and one for the parent.

Considering that the change that has motivated the discussion is the fact that it is impossible to flag geometries, I wonder if this is the way to go, that is to say, to derive the geometries from the flags. If technically possible, this would be in my opinion a much flexible solution.

RiccardoRossi commented 2 years ago

guys i am really not clear about this.

the element needs to point to the geometry who has the kinematics NOT to the element (who has the physics...)

rubenzorrilla commented 2 years ago

guys i am really not clear about this.

the element needs to point to the geometry who has the kinematics NOT to the element (who has the physics...)

Though I see your point, which is also fair to me, I think it would be nice to have access to the flags from the constitutive law. I think it's worth discussing how to achieve this.

pablobecker commented 2 years ago

About removing the pointer to the geometry I'm concerned about the fact that this will obligue to always define an element to use the constitutive law. This means that constitutive laws will no longer be compatible with conditions, something that it is right now technically possible. We need to be sure that this is the way we want to go.

Good point, i handt thought of that.

Considering that the change that has motivated the discussion is the fact that it is impossible to flag geometries, I wonder if this is the way to go, that is to say, to derive the geometries from the flags. If technically possible, this would be in my opinion a much flexible solution

But how would it work? you mean sharing the flags just like we do with the container, with all the elements that make use of this geometry?

On Tue, 22 Mar 2022 at 16:59, Rubén Zorrilla @.***> wrote:

guys i am really not clear about this.

the element needs to point to the geometry who has the kinematics NOT to the element (who has the physics...)

Though I see your point, which is also fair to me, I think it would be nice to have access to the flags from the constitutive law. I think it's worth discussing how to achieve this.

— Reply to this email directly, view it on GitHub https://github.com/KratosMultiphysics/Kratos/issues/9775#issuecomment-1075348479, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGD7TCPT7M5FFYUYTHZMRX3VBHU55ANCNFSM5Q6VTBGA . You are receiving this because you authored the thread.Message ID: @.***>

ddiezrod commented 2 years ago

@rubenzorrilla If I remember correctly, flags are in the element and not in the geometry on purpose. At some point, we discussed this and we arrived to the conclusion that different elements should have different flags even if they share the same geometry (for example you may want to have different values for flag ACTIVE)

miguelmaso commented 2 years ago

About removing the pointer to the geometry I'm concerned about the fact that this will obligue to always define an element to use the constitutive law. This means that constitutive laws will no longer be compatible with conditions, something that it is right now technically possible. We need to be sure that this is the way we want to go.

Maybe, a pointer to the geometrical object could solve this issue

rubenzorrilla commented 2 years ago

@rubenzorrilla If I remember correctly, flags are in the element and not in the geometry on purpose. At some point, we discussed this and we arrived to the conclusion that different elements should have different flags even if they share the same geometry (for example you may want to have different values for flag ACTIVE)

You're right. This is why we chose not flagging the geometries. Now I remember.

rubenzorrilla commented 2 years ago

About removing the pointer to the geometry I'm concerned about the fact that this will obligue to always define an element to use the constitutive law. This means that constitutive laws will no longer be compatible with conditions, something that it is right now technically possible. We need to be sure that this is the way we want to go.

Maybe, a pointer to the geometrical object could solve this issue

Agree. This would be much better solution IMO, the element/condition limitation I was referring to wouldn't apply in this case.

RiccardoRossi commented 2 years ago

we need to be very careful not to introduce circular dependencies...and honestly i see no way to avoid it.

also, what if someone wants to have a CL without element? (quite common if you are doing CL research)

loumalouomega commented 2 years ago

also, what if someone wants to have a CL without element? (quite common if you are doing CL research)

nullptr

RiccardoRossi commented 2 years ago

we have been discussing this in the @KratosMultiphysics/technical-committee .

Giving a pointer to element or to geometrical_object may open a Pandora's box of circular dependencies.

One simple solution to the problem that is being raised could be to add a "ElementalFlag" method to the cl_parameters. since flags are easily copied this should represent no issue. Furthermore this approach ensures that the CL cannot change the elemental flags (note that we are proposing to pass a copy of the elemental flags and not a pointer to it)

rubenzorrilla commented 2 years ago

Giving a pointer to element or to geometrical_object may open a Pandora's box of circular dependencies.

Just adding that with this means changing the element from the claw. Same situation would happen if pointing to the GeometricalObject.