Conversation
rfaasse
added
GeoMechanics
avdg81
left a comment
avdg81
left a comment
There was a problem hiding this comment.
Hi Richard,
Thanks a lot for resolving this bug. The change matches my expectation and the added unit test will ensure that the fix remains in place. To me, this PR is good to go. 👍
applications/GeoMechanicsApplication/custom_utilities/neighbouring_element_finder.cpp
Outdated
Show resolved
Hide resolved
| const auto& r_entities = it->second; | ||
| for (auto& rp_entity : r_entities) { | ||
| if (rp_entity->GetGeometry().Id() == pElement->GetGeometry().Id()) continue; | ||
| if (IsSameEntity(*pElement, *rp_entity)) continue; |
There was a problem hiding this comment.
a quick question, as far as I understand before this check skipped elements with the same geometry, now it skips the same type elements with the same geometry. Should it check for same element pointers? rp_entity.get() == pElement
There was a problem hiding this comment.
Before indeed we checked if the geometry (id) was the same, but we found that sometimes the geometries of a condition/element could be the same and we still would like them to be flagged as neighbours. Right now, we indeed check if the entities are of the same type and if they have the same id (the entity itself, so not the geometry).
A very good point about comparing the pointers, I hadn't thought about that possibility. I tested your suggestion and that also works (at least for the unit tests and python small suite) and I think it's slightly more neat, as it checks if the object is really the same. Assuming there are never copies of elements/conditions lying around (which I don't think is happening now), I think it's the cleaner option.
There was a problem hiding this comment.
The nightly and validation tests were also fine after implementing the simpler and cleaner check, thanks a lot for the suggestion!
avdg81
left a comment
avdg81
left a comment
There was a problem hiding this comment.
I feel this PR is good to go 👍
3 participants
📝 Description
An issue was found where if an element and condition have the same geometry, they are not flagged as neighbours. The check is improved now to avoid this issue.