Fix open boundaries with periodic boundary zone#1237
Conversation
efaulhaber
left a comment
There was a problem hiding this comment.
Codex Review
The PR updates open-boundary particle classification and transfer to use periodic coordinates, with a regression test covering conversion near a periodic boundary. I did not identify correctness or regression issues in the changed production behavior.
This is an AI-generated code review. Please verify the findings and summary before acting on them.
Review generated by codex-pr-review
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1237 +/- ##
=======================================
Coverage 90.13% 90.13%
=======================================
Files 138 138
Lines 10797 10801 +4
=======================================
+ Hits 9732 9736 +4
Misses 1065 1065
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| particle_coords = current_coords(u_old, system_old, particle_old) | ||
| particle_coords = PointNeighbors.periodic_coords(particle_coords, periodic_box) |
There was a problem hiding this comment.
these coords should have different names
There was a problem hiding this comment.
or just move the first line inside like in the other places
| @test TrixiParticles.is_in_boundary_zone(outflow, | ||
| SVector(Tuple(u_open_boundary[:, | ||
| particle_new]))) |
There was a problem hiding this comment.
| @test TrixiParticles.is_in_boundary_zone(outflow, | |
| SVector(Tuple(u_open_boundary[:, | |
| particle_new]))) | |
| @test TrixiParticles.is_in_boundary_zone( | |
| outflow, | |
| PointNeighbors.periodic_coords(SVector(Tuple(u_open_boundary[:, particle_new])), | |
| periodic_box) | |
| ) |
| particle_coords = PointNeighbors.periodic_coords(current_coords(u, system, particle), | ||
| periodic_box) |
There was a problem hiding this comment.
| particle_coords = PointNeighbors.periodic_coords(current_coords(u, system, particle), | |
| periodic_box) |
| v, u, v_fluid, u_fluid) | ||
| v, u, v_fluid, u_fluid, periodic_box) | ||
| # Position relative to the origin of the transition face | ||
| relative_position = current_coords(u, system, particle) - boundary_zone.zone_origin |
There was a problem hiding this comment.
| relative_position = current_coords(u, system, particle) - boundary_zone.zone_origin | |
| particle_coords = PointNeighbors.periodic_coords(current_coords(u, system, particle), | |
| periodic_box) | |
| relative_position = current_coords(u, system, particle) - boundary_zone.zone_origin |
When simulating periodic boundaries top/bottom and open boundaries left/right, the boundary zones are periodic in tangential direction. This doesn't work on main, as fluid particles that passed through the periodic boundary (in the screenshot below particles from the bottom that appear at the top) don't get converted when entering the boundary zone.
Note the orphaned fluid particles at the top in this screenshot.
