Filter degenerate triangles #42
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maresb
force-pushed
the
filter-degenerate-triangles
branch
4 times, most recently
from
e77e86d to
0e8b9ee
Compare
- Add comprehensive tests for degenerate triangle handling - Test both no-NaN and zero-result properties for degenerate triangles - Cover all degenerate cases: coincident vertices, collinear points, duplicates, nearly degenerate - Test both fullspace and halfspace modules with generic observation points and slip vectors - Ensures robust handling of edge cases in triangular dislocation element calculations
- Add triangle_area() function to compute triangle area using cross product - Add is_degenerate_triangle() function to detect triangles with zero area - Functions placed after cross3() and length3() to ensure dependencies are available - Foundation for handling degenerate triangles in displacement/strain calculations
- Extract disp_fs_core() macro containing displacement computation logic - Extract strain_fs_core() macro containing strain computation logic - Simplify disp_fs() to use setup_tde() + disp_fs_core() pattern - Simplify strain_fs() to use setup_tde() + strain_fs_core() pattern - Parent functions declare full_out variable, core functions assign to it - Pure refactoring: no behavior change, prepares for degeneracy checks
- Add parametrized harmonic_fsc macro for both displacement and strain calculations - Replace duplicated FSC code with reusable macro - Use parameters to specify AngSetupFSC vs AngSetupFSC_S and add3 vs add6 - Declare fsc_term in calling scope to avoid variable scoping issues - Pure refactoring with no functional changes
- Add setup_image_triangle macro for halfspace method of images calculations - Creates mirror image triangles by negating z-coordinates for free surface reflection - Parametrize source and destination triangle prefixes for flexibility - Declare image triangle variables in calling scope to avoid scoping issues - Replace duplicated image triangle creation code in disp_hs and strain_hs - Pure refactoring with no functional changes
The current implementation has a fundamental scoping issue: halfspace calculations need to perform both fullspace calculations and harmonic free surface corrections using the same TDE coordinate system. However, calling the full disp_fs/strain_fs functions means the TDE coordinate system (Vnorm, Vstrike, Vdip) is established and then immediately goes out of scope, making it unavailable for subsequent FSC calculations. This restructures the halfspace functions to: - Call setup_tde once at the beginning to establish the TDE coordinate system - Use disp_fs_core and strain_fs_core for the actual calculations - Keep the coordinate system in scope for both main and FSC calculations - Properly declare full_out variables in appropriate scopes This architectural change enables code reuse and prepares for additional calculations that need access to the established TDE coordinate system.
Implement degeneracy checks in disp_fs, strain_fs, disp_hs, and strain_hs functions to return zero displacements/strains for degenerate triangles. This prevents NaN values and division by zero errors when dealing with triangles that have zero area (collinear or coincident vertices).
maresb
force-pushed
the
filter-degenerate-triangles
branch
from
0e8b9ee to
d218101
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR implements filtering for degenerate triangles to prevent NaN values in displacement and strain calculations. The changes ensure that triangles with zero area (such as coincident vertices, collinear points, or duplicate vertices) return zero displacement/strain instead of producing NaN results.
- Adds helper functions to detect degenerate triangles based on area calculation
- Modifies displacement and strain computation functions to check for degeneracy and return zero values
- Refactors existing template code to separate core computation from degeneracy handling
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| cutde/common.cu | Implements triangle area calculation, degeneracy detection, and updates displacement/strain functions with early returns for degenerate triangles |
| tests/test_tde.py | Adds comprehensive test suite covering various degenerate triangle scenarios and mixed degenerate/non-degenerate cases |
|
|
||
| WITHIN_KERNEL int is_degenerate_triangle(Real3 tri0, Real3 tri1, Real3 tri2) { | ||
| Real area = triangle_area(tri0, tri1, tri2); | ||
| return area == 0; |
There was a problem hiding this comment.
Using exact equality comparison with floating-point values is problematic. Consider using a small epsilon tolerance instead: return area < epsilon; where epsilon is a small value like 1e-15.
Collaborator
Author
There was a problem hiding this comment.
I expect this to be fine. If the area is even slightly positive, then the cross product was nonzero and therefore will be normalizable, and the computation should be able to proceed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Triangles with zero area should result in displacement/strain of 0, not NaN.