Skip to content
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

Repeated Identical Geometries in mdpa #12564

Open
jginternational opened this issue Jul 19, 2024 · 13 comments · May be fixed by #12703
Open

Repeated Identical Geometries in mdpa #12564

jginternational opened this issue Jul 19, 2024 · 13 comments · May be fixed by #12703

Comments

@jginternational
Copy link
Member

jginternational commented Jul 19, 2024

@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 KratosMultiphysics/GiDInterface#990 (comment)

@matekelemen
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member

RiccardoRossi commented Jul 28, 2024 via email

@RiccardoRossi
Copy link
Member

RiccardoRossi commented Jul 28, 2024 via email

@rubenzorrilla
Copy link
Member

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
Copy link
Member

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
Copy link
Member

@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
Copy link
Member

@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
Copy link
Member Author

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:

  • This will end up in the same inefficiency code we are replacing, This kind of checks in tcl are a scalability problem, while doing it in a c++ is the optimal.

  • In any case, this check is done in Kratos because, one could modify the gid mdpa output by hand and get the same 'problem' so a human is also a preprocessor.

  • For me, the "silent check" for identical id and connectivities and rise an error in the case of 'same id different connectivities' is fine.

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
Copy link
Member

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
Copy link
Contributor

matekelemen commented Jul 31, 2024

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
Copy link
Member Author

jginternational commented Aug 3, 2024

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
Copy link
Member

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 :

NodesContainerType::iterator existing_node_it = this->GetMesh(ThisIndex).Nodes().find(Id);
if( existing_node_it != GetMesh(ThisIndex).NodesEnd())
{
//the node already exists - now check if the position we ask for coincides with the one of the existing one
double distance = std::sqrt( std::pow( existing_node_it->X() - x,2) + std::pow(existing_node_it->Y() - y,2) + std::pow(existing_node_it->Z() - z,2) );
KRATOS_ERROR_IF(distance > std::numeric_limits<double>::epsilon()*1000)
<< "trying to create a node with Id " << Id << " however a node with the same Id already exists in the root model part. Existing node coordinates are " << existing_node_it->Coordinates() << " coordinates of the nodes we are attempting to create are :" << x << " " << y << " " << z;
//if the node we attempt to create is in the same position as the one that is already there, we return the old one
return *(existing_node_it.base());
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants