feature: integrate rotations into ICPP patches (#1594)#1612
Conversation
SVS87
force-pushed
the
feature/icpp-patch-rotations
branch
2 times, most recently
from
423d5f1 to
f0332b3
Compare
Add a rotation framework to initial-condition (ICPP) patches, mirroring the existing immersed-boundary rotation support. - Extract the rotation-matrix computation into a new shared module m_patch_geometries, taking angle/matrix arrays as arguments so both ICPP and IB patches can call it (m_ib_patches now uses a thin wrapper). - Add angles + rotation_matrix(_inverse) fields to ic_patch_parameters, defaulting angles to zero (identity rotation) so existing cases are unaffected. - Apply the inverse-rotation transform in the rectangle inside-test as a proof of concept; remaining geometries are straightforward follow-up. - Register patch_icpp(i)%angles in the parameter definitions so the angle is settable from a case file. - Add a 2D_rotated_rectangle example demonstrating a 45-degree patch.
SVS87
force-pushed
the
feature/icpp-patch-rotations
branch
from
f0332b3 to
c4dea86
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1612 +/- ##
==========================================
+ Coverage 60.51% 60.53% +0.01%
==========================================
Files 83 83
Lines 19905 19913 +8
Branches 2950 2950
==========================================
+ Hits 12046 12054 +8
Misses 5866 5866
Partials 1993 1993 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
danieljvickers
left a comment
danieljvickers
left a comment
There was a problem hiding this comment.
These are pretty much all the same changes I would have made. Well done. Two notes before this should be merged:
- I assume you are still working on integrating this with all of the other icpp patch geometries. This should be integrated with all of them before this can be merged. The STL geometry will require some special attention. Just copy what is in the m_ib_patches.fpp file.
- The wrapper function in simulation around the s_compute_rotation_matrix subroutine is necessary. It adds nothing meaningful and should be removed.
There was a problem hiding this comment.
Not sure about this name, as it could get confused with IB patch code. Maybe speciify it is a fluid patch?
There was a problem hiding this comment.
Changing it to 2D_rotated_fluid_rectangle. Would that work?
| real(wp), dimension(1:3,1:3), intent(out) :: rotation_matrix | ||
| real(wp), dimension(1:3,1:3), intent(out) :: rotation_matrix_inverse | ||
| real(wp), dimension(3, 3, 3) :: rotation | ||
| real(wp) :: angle |
There was a problem hiding this comment.
Is there a reason to add this additional angle value?
There was a problem hiding this comment.
Removed it, it was unnecessary. Updated change just uses angles(1, 2, 3) respectively
| real(wp), dimension(1:3), intent(in) :: angles | ||
| real(wp), dimension(1:3,1:3), intent(out) :: rotation_matrix | ||
| real(wp), dimension(1:3,1:3), intent(out) :: rotation_matrix_inverse | ||
| real(wp), dimension(3, 3, 3) :: rotation |
There was a problem hiding this comment.
Making this 3x3x3 is a nice touch
| ! Assign patch vars if cell is covered and patch has write permission | ||
| do j = 0, n | ||
| do i = 0, m | ||
| if (f_is_inside_cuboid(x_cc(i) - x_centroid, y_cc(j) - y_centroid, 0._wp, [length_x, length_y, 0._wp])) then |
There was a problem hiding this comment.
I see where you added cuboids. I assume you are in the process of adding this to the remaining geometries? That is fine if so. It just needs to be integrated with all of them before merge.
| @@ -415,45 +415,10 @@ contains | |||
There was a problem hiding this comment.
This wrapper function is now unnecessary. You should just modify the code everywhere to take the new rotation matrix subroutine and delete this subroutine all together.
…ation wrapper - Extend rotation transform to ellipse, ellipsoid, cuboid, cylinder, TaylorGreen, and STL (mirroring m_ib_patches.fpp; omits IB-only centroid_offset) - Skip circle/sphere as rotation-invariant - Remove unused angle variable in s_compute_rotation_matrix - Delete s_update_ib_rotation_matrix wrapper; callers now call s_compute_rotation_matrix directly - Rename example to 2D_rotated_fluid_rectangle to clarify it is an ICPP patch
…87/MFC into feature/icpp-patch-rotations
- Add model_rotate parameter to stl_models (parallel to model_scale/model_translate), applied at model load in s_instantiate_STL_models so STL geometry rotates in world coordinates - Wire model_rotate through derived type, defaults, MPI broadcast, and toolchain registration - Verified: a cube rotated 45 degrees about z expands its x/y bounding box by sqrt(2) as expected - Extend per-cell rotation to remaining ICPP geometries (ellipse, ellipsoid, cuboid, cylinder, TaylorGreen)
2 participants
Description
Add a rotation framework to initial-condition (ICPP) patches, mirroring the existing immersed-boundary rotation support.
Part of #1594
Type of change (delete unused ones)
Testing
behavior-neutral for all existing cases.
the shared s_compute_rotation_matrix, confirming the extracted routine
matches the original.
output shows the rectangle correctly tilted (image below).
Checklist
Check these like this
[x]to indicate which of the below applies.See the developer guide for full coding standards.
GPU changes (expand if you modified
src/simulation/)Verified CPU only
AI code reviews
Reviews are not retriggered automatically. To request a review, comment on the PR:
@claude full review— Claude full review (also triggers on PR open/reopen/ready)claude-full-review— Claude full review via label