Skip to content

[CORE] Fix IsInside of Truss_3D_2#14304

Open
Igarizza wants to merge 5 commits intomasterfrom
core/fix_line_3d_2_IsInside
Open

[CORE] Fix IsInside of Truss_3D_2#14304
Igarizza wants to merge 5 commits intomasterfrom
core/fix_line_3d_2_IsInside

Conversation

@Igarizza
Copy link
Copy Markdown
Member

@Igarizza Igarizza commented Mar 17, 2026

📝 Description
Fix the Point Projection onto the truss element in the 3D case by adding the distance check.

Corner Case:

Point (-16, 2.5, 0)

r_element : Element #21 :
Working space dimension : 3
Local space dimension : 1

Point 1	 :  (-20, 0, 0)
Dofs :
    Free TEMPERATURE degree of freedom
    Free DISPLACEMENT_X degree of freedom
    Free DISPLACEMENT_Y degree of freedom
    Free DISPLACEMENT_Z degree of freedom

Point 2	 :  (-20, 5, 0)
Dofs :
    Free TEMPERATURE degree of freedom
    Free DISPLACEMENT_X degree of freedom
    Free DISPLACEMENT_Y degree of freedom
    Free DISPLACEMENT_Z degree of freedom

Center	 :  (-20, 2.5, 0)

rResult[0] : 0.886796

Expected to be False, Returns True.

@Igarizza Igarizza requested a review from sunethwarna March 17, 2026 17:40
@Igarizza Igarizza requested a review from a team as a code owner March 17, 2026 17:40
@Igarizza Igarizza self-assigned this Mar 18, 2026
@Igarizza Igarizza changed the title [CORE] Fix Point Projection onto Truss_3D_2 [CORE] Fix IsInside of Truss_3D_2 Mar 18, 2026
sunethwarna
sunethwarna previously approved these changes Mar 19, 2026
Copy link
Copy Markdown
Member

@sunethwarna sunethwarna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Igarizza :)

@philbucher
Copy link
Copy Markdown
Member

FYI not sure this is the way to go, check #7401

=> IMO the projection should happen separately (see geom-projection-utils)

@Igarizza
Copy link
Copy Markdown
Member Author

FYI not sure this is the way to go, check #7401

=> IMO the projection should happen separately (see geom-projection-utils)

Well, I have revered the projection and only adding the distance check. For sure it makes the function more expansive.

@matekelemen
Copy link
Copy Markdown
Contributor

FYI not sure this is the way to go, check #7401

=> IMO the projection should happen separately (see geom-projection-utils)

Wow this topic is old. How on earth did this never get merged?!

@philbucher
Copy link
Copy Markdown
Member

Wow this topic is old. How on earth did this never get merged?!

tests were failing, and I guess not enough interest to fix it 🤷

@Igarizza
Copy link
Copy Markdown
Member Author

Igarizza commented Apr 7, 2026

@sunethwarna In CoSimApp, the PointLocalCoordinates is used to project a faraway point on the truss elements for beam mapping cases. Adding a distance check kills this function completely. I can move the check distance to IsInside as it was originally. What do you think?

Снимок экрана 2026-04-07 в 09 28 49

Copilot AI review requested due to automatic review settings April 7, 2026 07:56
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Line3D2::IsInside for 3D truss/line geometries by rejecting points that are within the parametric range but far away from the actual line segment (regression case in the PR description).

Changes:

  • Add a perpendicular distance check to Line3D2::IsInside to avoid false positives for off-line points.
  • Add a regression test covering the reported corner case in test_line_3d_2.cpp.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
kratos/geometries/line_3d_2.h Adds a distance-to-segment check in IsInside to prevent off-line points being classified as inside.
kratos/tests/cpp_tests/geometries/test_line_3d_2.cpp Adds a new corner-case geometry + assertion to prevent regressions of the reported bug.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

// Check if point is too far away from the line, if so, return false
const double distance = this->CalculateDistance(rPoint, Tolerance);
const double length = Length();
if (distance > std::max(Tolerance, 1e-6 * length)) {
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tolerance in IsInside is used as a local-space boundary tolerance (dimensionless), but here it is also compared directly against a global-space distance. This mixes units and can make the “allowed” perpendicular distance unexpectedly large/small depending on the caller-provided Tolerance. Consider using a distance tolerance expressed in length units (e.g., scale Tolerance by Length() or follow the existing Line2D2 approach using a relative 1e-6*Length() threshold) and avoid using the raw Tolerance value as a distance.

Suggested change
if (distance > std::max(Tolerance, 1e-6 * length)) {
const double distance_tolerance = std::max(Tolerance * length, 1e-6 * length);
if (distance > distance_tolerance) {

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For better copilot , related: #14334 @roigcarlo ?

@Igarizza Igarizza requested a review from sunethwarna April 7, 2026 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants