-
Notifications
You must be signed in to change notification settings - Fork 132
Levelset refactor #1123
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
Levelset refactor #1123
Changes from all commits
89a0ac5
ef446f5
2f9b1be
e3495d9
f23eaff
59c8dc2
b7b8d2d
128562b
3e097d7
285999f
455d16b
d5aa186
ed9edac
4fa238a
97c54f3
0662d47
75ddc11
b2a821c
6c45fc6
2b84f06
e8fd89f
fb9dbaf
a9c6c5c
31799e1
070e062
8c4fa75
12256ec
0497b85
2a0fb21
d7bef7f
8427e56
7980cb5
1781f45
30dd642
4d69e5a
13cfbbd
aafff97
cc67478
b56678e
dbef122
b7978a3
d8432f6
e7e7d33
b7b6284
05915fb
51bc07f
c304b37
6d7f39c
05c069e
d363ed6
4e0553b
0002855
dc65fc7
668604e
3eb260d
5c9ff7e
1701344
7a9a435
47c9ced
baca16e
20774c6
3769f12
5b5f170
0975fd3
76ab0db
ccca21f
89412b6
c0c9ea3
99de417
7f21870
a2ebac7
aef9016
fe0dfab
9952603
ee7e431
3932983
9c58276
c0948ea
f0d8d5e
0041c4a
8ea881d
4ac3e67
a6d2048
375e324
0c138b7
a91ecca
326f7df
508952b
feae160
5d1ebe8
2cdea4c
4f2f6dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -180,8 +180,18 @@ module m_derived_types | |||||||||
| type :: t_model | ||||||||||
| integer :: ntrs ! Number of triangles | ||||||||||
| type(t_triangle), allocatable :: trs(:) ! Triangles | ||||||||||
|
|
||||||||||
| end type t_model | ||||||||||
|
|
||||||||||
| type :: t_model_array | ||||||||||
| type(t_model), allocatable :: model | ||||||||||
| real(wp), allocatable, dimension(:, :, :) :: boundary_v | ||||||||||
| real(wp), allocatable, dimension(:, :) :: interpolated_boundary_v | ||||||||||
| integer :: boundary_edge_count | ||||||||||
| integer :: total_vertices | ||||||||||
| logical :: interpolate | ||||||||||
| end type t_model_array | ||||||||||
danieljvickers marked this conversation as resolved.
Show resolved
Hide resolved
danieljvickers marked this conversation as resolved.
Show resolved
Hide resolved
danieljvickers marked this conversation as resolved.
Show resolved
Hide resolved
danieljvickers marked this conversation as resolved.
Show resolved
Hide resolved
danieljvickers marked this conversation as resolved.
Show resolved
Hide resolved
danieljvickers marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
|
|
||||||||||
| !> Derived type adding initial condition (ic) patch parameters as attributes | ||||||||||
| !! NOTE: The requirements for the specification of the above parameters | ||||||||||
| !! are strongly dependent on both the choice of the multicomponent flow | ||||||||||
|
|
@@ -437,6 +447,8 @@ module m_derived_types | |||||||||
| integer, dimension(3) :: ip_grid !< Top left grid point of IP | ||||||||||
| real(wp), dimension(2, 2, 2) :: interp_coeffs !< Interpolation Coefficients of image point | ||||||||||
| integer :: ib_patch_id !< ID of the IB Patch the ghost point is part of | ||||||||||
| real(wp) :: levelset | ||||||||||
| real(wp), dimension(1:3) :: levelset_norm | ||||||||||
|
Comment on lines
+450
to
+451
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The newly added Severity Level: Critical 🚨- ❌ Ghost-point classification may be incorrect.
- ❌ Levelset-based IB forces/markers miscomputed.
- ⚠️ Simulation results nondeterministic across runs.
Suggested change
Steps of Reproduction ✅1. Inspect the ghost_point type in src/common/m_derived_types.fpp lines 448-455; lines
451-452 add levelset and levelset_norm fields.
2. Create a minimal reproducer (or find IB code that allocates ghost points) that declares
a ghost_point variable and reads the fields before any assignment:
- use m_derived_types
- type(ghost_point) :: g
- print *, g%levelset, g%levelset_norm
3. Compile and run the reproducer. The printed values will be indeterminate
(uninitialized), demonstrating that reading these fields without prior initialization
yields garbage or non-deterministic values.
4. In a real simulation run, any IB/ghost-point routine that reads g%levelset or
g%levelset_norm for classification or interpolation before explicitly writing them (for
example, ghost-point setup or levelset-based boundary condition code paths) will make
wrong decisions; verify by locating IB routines that use ghost_point fields after
allocation and before explicit assignment.Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** src/common/m_derived_types.fpp
**Line:** 451:452
**Comment:**
*Possible Bug: The newly added `levelset` and `levelset_norm` fields in `ghost_point` are not initialized, so any code that reads these values before they are explicitly set (e.g., for classification or interpolation) will operate on garbage data and may misclassify ghost points.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. |
||||||||||
| logical :: slip | ||||||||||
| integer, dimension(3) :: DB | ||||||||||
| end type ghost_point | ||||||||||
|
|
||||||||||
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.
Suggestion: The component
modelint_model_arrayis declared as a scalar allocatable rather than an allocatable array, which prevents this type from correctly representing multiple models; any calling code that expects to store or iterate over several models in this container will either silently only store one model or malfunction when trying to index it. [logic error]Severity Level: Major⚠️
Steps of Reproduction ✅
Prompt for AI Agent 🤖