-
Notifications
You must be signed in to change notification settings - Fork 277
[Core][Geometries] Adding to base class GetIntersectionPoints #10504
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
base: master
Are you sure you want to change the base?
[Core][Geometries] Adding to base class GetIntersectionPoints #10504
Conversation
ddiezrod
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments, I do not like that the code is duplicated in both classes. but I guess a possible fix for that would go much further than the scope of this PR. We could maybe have a line and triangle base classes to avoid such duplications
Co-authored-by: Daniel Diez <ddiez@europe.altair.com>
Co-authored-by: Daniel Diez <ddiez@europe.altair.com>
|
We were discussing this today in @KratosMultiphysics/implementation-committee , in principle we agree this is a nice addition to the geometry classes. Only please, to avoid code duplication, try to move the common code to intersection_utilties. @loumalouomega |
|
Please refer to the #10470 for the discussion we made about this one. |
|
@pooyan-dadvand can we delegate decisions on this to you? we guess you are following this more closely with Vicente |
|
@pooyan-dadvand ping, can you take this over? |
Hello |
📝 Description
Closes #10470. WE have the
HasIntersectionmethod, but no method to get the actual intersection points. This method adds the corresponding method is some geometries (where the intersection is already implemented as auxiliary utilities). Using a std::vector, so the number of intersection points can be arbitrary, between 0 (no intersection), to the required size for the specific geometry combination🆕 Changelog