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.02k stars 244 forks source link

Repeated Identical Geometries in mdpa #12564

Open jginternational opened 2 months ago

jginternational commented 2 months ago

@rubenzorrilla : After having discussed this with @jginternational we concluded that the issue here is having something similar to

Begin Geometries Triangle2D3
    1 10 20 30
    1 10 20 30
    2 20 30 40
    ....
End Geometries

that is having the same entity repeated. Note that this is to happen in many cases (for instance having a shell with surface load conditions). Right now, we throw an error if the same geometry id is present, but the error should in our opinion be thrown if the same id with different connectivities is present. If the same id with same connectivities is only repeated information which should be simply skipped. @KratosMultiphysics/technical-committee

Originally posted by @rubenzorrilla in https://github.com/KratosMultiphysics/GiDInterface/issues/990#issuecomment-2239182961

matekelemen commented 2 months ago

If the same id with same connectivities is only repeated information which should be simply skipped.

Why? Such cases would probably result from a buggy MDPA writer and should be fixed rather than adding complexity to the MDPA reader.

rubenzorrilla commented 2 months ago

If the same id with same connectivities is only repeated information which should be simply skipped.

Why? Such cases would probably result from a buggy MDPA writer and should be fixed rather than adding complexity to the MDPA reader.

Not necessarily. Depends on the preprocessor, something that cannot be controlled from our side. Note that the complexity is already there, as right now we are checking for the repeated IDs.

RiccardoRossi commented 2 months ago

Well, we may want to repertorio one geoentry...

El dom, 28 jul 2024, 10:30, Rubén Zorrilla @.***> escribió:

If the same id with same connectivities is only repeated information which should be simply skipped.

Why? Such cases would probably result from a buggy MDPA writer and should be fixed rather than adding complexity to the MDPA reader.

Not necessarily. Depends on the preprocessor, something that cannot be controlled from our side. Note that the complexity is already there, as right now we are checking for the repeated IDs.

— Reply to this email directly, view it on GitHub https://github.com/KratosMultiphysics/Kratos/issues/12564#issuecomment-2254393694, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB5PWEN7SQLGDGP4EL7CZXTZOST3DAVCNFSM6AAAAABLETP5X6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJUGM4TGNRZGQ . You are receiving this because you are on a team that was mentioned.Message ID: @.***>

RiccardoRossi commented 2 months ago

I have been thinking about this.

Indeed repente geometrías should not be an expected usecase. We would duplicate them ourselves if we need.

Having said this i am not sure if i would throw an error or a warning. From the coding point of view it is the same effort...Just not sure about what i would pick

El dom, 28 jul 2024, 10:44, Riccardo Rossi @.***> escribió:

Well, we may want to repertorio one geoentry...

El dom, 28 jul 2024, 10:30, Rubén Zorrilla @.***> escribió:

If the same id with same connectivities is only repeated information which should be simply skipped.

Why? Such cases would probably result from a buggy MDPA writer and should be fixed rather than adding complexity to the MDPA reader.

Not necessarily. Depends on the preprocessor, something that cannot be controlled from our side. Note that the complexity is already there, as right now we are checking for the repeated IDs.

— Reply to this email directly, view it on GitHub https://github.com/KratosMultiphysics/Kratos/issues/12564#issuecomment-2254393694, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB5PWEN7SQLGDGP4EL7CZXTZOST3DAVCNFSM6AAAAABLETP5X6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJUGM4TGNRZGQ . You are receiving this because you are on a team that was mentioned.Message ID: @.***>

rubenzorrilla commented 2 months ago

Note that these are not repeated geometries but repeated information of the same geometry. The case we are discussing in here is same ID and same connectivities. Repeated geometries would be different ID but same connectivities which should IMO also be a valid case (note that this is what we've been doing so far with the rest of entities). In other words, the unique thing that necessarily needs to be unique is the ID, meaning that the error must be thrown if the same ID applies to different geometries that is to say, the same ID applies to different connectivities.

philbucher commented 2 months ago

my two $: Disallow anything duplicated by default, but given this is the historical behavior there should be a flag that lets the user enable it Or disallow it entirely from the beginning, geometries are new in the mdpa, why clutter it from the beginning

RiccardoRossi commented 2 months ago

@KratosMultiphysics/technical-committee decided that we shall silently skip "safe" cases, that is skipping adding a geometry if it is added more than once (same id, same connectivities).

We shall also add an optional flag to the model_part_io to eventually throw a warning or error if the user wants to do so. (default is silently ignoring it)

@philbucher the problem here is that receiving duplicated lines is out of our control since it is provided by the preprocessor. Luckily this can be handled safely since we can do an exact check given that ids are integers.

philbucher commented 2 months ago

@philbucher the problem here is that receiving duplicated lines is out of our control since it is provided by the preprocessor. Luckily this can be handled safely since we can do an exact check given that ids are integers.

Isnt gid the only one that writes mdpa?

jginternational commented 2 months ago

just to add some background here, From the GiD Problemtype, we could try to handle this "repeated id and connectivities", it's a simple code but:

Sorry if my explanation is not very clear, I'm not sleeping well and my mind is set on my newborn poop developer 👶 (hopefully not a TCL developer)

loumalouomega commented 2 months ago

Sorry if my explanation is not very clear, I'm not sleeping well and my mind is set on my newborn poop developer 👶 (hopefully not a TCL developer)

So cute <3

matekelemen commented 2 months ago

I don't understand. Does the internal mesher of GiD produce repeated geometries?

As an example of the many headaches that being lenient on the input side would entail, allowing geometries with identical connectivities would mean allowing index rotations as well (e.g.: a linear triangle 1,2,3 is equivalent to 2,3,1). However, such identities depend on the geometry type (for linear geometries it's just a rotation of a list of IDs, but for higher order geometries it's more complicated - e.g.: a quadratic triangle 1,2,3,4,5,6 is identical to 2,3,1,5,6,4 but not 2,3,4,5,6,1) and that would make life unnecessarily complicated.

Unless the problem is unreasonably difficult/expensive to solve on the MDPA output side, I'd rather be more strict with reading and forbid repeated IDs, regardless of the actual connectivities. So the real question is: how or why does GiD produce repeated geometries?

jginternational commented 1 month ago

Hi @matekelemen TLDR; The geometries are not repeated inside GiD, they are just used several times.

The current way to print the GiD mesh into MDPA uses the concept of groups. Examples: You can have a surface with a material applied and some conditions applied on it, like a pressure. The triangle mesh is the same, but since we are using it in different places, we are printing it several times.

Or applying several conditions over the same line. If the user has created only one group with a line, and applies multiple conditions on that group, it will be printed only once, but if the user creates a group for each condition, even if all the groups are identical, the same geometry is printed for every group.

As I say, it's trivial to do the check in the write script but remember that scripting languages have performance issues on big data volumes. I just want to avoid this ìs_writen` hashmap in TCL.

I understand that the check of the orientation can be a problem there.

I'm marking this issue to check if I can handle it somehow in the GiD C++ code, but it will be in September

pooyan-dadvand commented 1 week ago

I don't understand. Does the internal mesher of GiD produce repeated geometries?

As an example of the many headaches that being lenient on the input side would entail, allowing geometries with identical connectivities would mean allowing index rotations as well (e.g.: a linear triangle 1,2,3 is equivalent to 2,3,1). However, such identities depend on the geometry type (for linear geometries it's just a rotation of a list of IDs, but for higher order geometries it's more complicated - e.g.: a quadratic triangle 1,2,3,4,5,6 is identical to 2,3,1,5,6,4 but not 2,3,4,5,6,1) and that would make life unnecessarily complicated.

Unless the problem is unreasonably difficult/expensive to solve on the MDPA output side, I'd rather be more strict with reading and forbid repeated IDs, regardless of the actual connectivities. So the real question is: how or why does GiD produce repeated geometries?

@KratosMultiphysics/technical-committee we talked about your concern and to our understanding these connectivities are different. (for example 1,2,3 is not equivalent to 2,3,1) as the local coordinate system is different even though the normal orientaion is the same. So, we agree that it should send an error because we are assigning two different connectivites with same id. Nevertheless, if the order of ids are exactly the same, then we can safely ignore one of them:

// id n1 n2 n3
2 1 2 3
2 1 2 3 // ignored without error
3 1 2 3
3 2 3 1 // error

This is coherent with what we do with nodes. We ignore the repeated node in the same coordinate :https://github.com/KratosMultiphysics/Kratos/blob/6222e4d5c459566397495a22e40f6a4e5f573ca6/kratos/sources/model_part.cpp#L329-L340