Use fluid system kernel instead of boundary system kernel #880
Conversation
This reverts commit 9657150.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #880 +/- ##
==========================================
- Coverage 70.96% 70.88% -0.08%
==========================================
Files 108 108
Lines 7047 7059 +12
==========================================
+ Hits 5001 5004 +3
- Misses 2046 2055 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…Particles.jlOpen into fix_wrong_nhs_at_bnd
|
@efaulhaber dam_break_2d with float32 fails with allocations. I don't know why? |
|
/run-gpu-tests |
Co-authored-by: Erik Faulhaber <[email protected]>
|
/run-gpu-tests |
| function inner_compute_correction_values!(system::BoundarySystem, | ||
| neighbor_system::BoundarySystem, semi, u_ode, | ||
| v_ode, system_coords) | ||
| # This is not needed |
There was a problem hiding this comment.
Why not?
- What about open boundaries?
- Why ignore boundary particles? What about boundary model
SummationDensity? Then the sum is computed over all neighbors (including boundary neighbors), and if we want to use a corrected kernel, why should boundary neighbors be ignored?
There was a problem hiding this comment.
- OpenBoundarySPHSystem is not a BoundarySystem
- This requires a lot of changes for a feature that is not used much :S. I also didn't get around doing the tests...
| boundary_particle_spacing=particle_spacing, | ||
| smoothing_length=smoothing_length, | ||
| boundary_density_calculator=SummationDensity(), | ||
| boundary_density_calculator=fluid_density_calculator, |
There was a problem hiding this comment.
Why? I thought the point of the test was to have a SummationDensity boundary.
There was a problem hiding this comment.
Most of the time we use the same density calculators for both the boundary and the fluid.
The test still has summation_density for both as a test.
| # Some correction methods require very small time steps at the beginning of the simulation. | ||
| # An adaptive time integrator makes this easier and faster. | ||
| sol = solve(ode, RDPK3SpFSAL35(), save_everystep=false, callback=callbacks) | ||
| sol = solve(ode, RDPK3SpFSAL35(), dt=1e-8, save_everystep=false, callback=callbacks) |
There was a problem hiding this comment.
Why this extremely small initial time step?
There was a problem hiding this comment.
This test is flawed it was already broken on main.
…Particles.jlOpen into fix_wrong_nhs_at_bnd
…Particles.jlOpen into fix_wrong_nhs_at_bnd
2 participants
Non-breaking part of removing the kernel from the boundary.