Skip to content

Fix the validation setup based on Marrone2011 #858

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

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

svchb
Copy link
Collaborator

@svchb svchb commented Jul 9, 2025

This fixes the setup as provided by Marrone:
image

@svchb svchb requested review from efaulhaber and LasNikas July 9, 2025 12:46
@svchb svchb self-assigned this Jul 9, 2025
@svchb svchb added the bug Something isn't working label Jul 9, 2025
Copy link

codecov bot commented Jul 9, 2025

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 70.25%. Comparing base (59e3c98) to head (47e57d4).

Files with missing lines Patch % Lines
src/schemes/boundary/system.jl 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #858   +/-   ##
=======================================
  Coverage   70.25%   70.25%           
=======================================
  Files         106      106           
  Lines        6906     6906           
=======================================
  Hits         4852     4852           
  Misses       2054     2054           
Flag Coverage Δ
unit 70.25% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines +22 to +69
function pressure_probe(coord_top, coord_bottom, v_ode, u_ode, t, system, semi) end

function pressure_probe(coord_top, coord_bottom, v_ode, u_ode, t,
system::TrixiParticles.BoundarySystem, semi)
# The sensor is at the right wall, so its x-coordinate is the same for top and bottom.
x_sensor = coord_top[1]

# Use the initial particle spacing as a reference for the thickness of the averaging region.
# A thickness of one or two particle spacings is usually a good choice.
region_thickness = 2.0 * particle_spacing

# Define the rectangular region for averaging
x_min = x_sensor - region_thickness
x_max = x_sensor + region_thickness
y_min = coord_bottom[2]
y_max = coord_top[2]

sum_of_pressures = 0.0
num_particles_in_region = 0

v = TrixiParticles.wrap_v(v_ode, system, semi)
u = TrixiParticles.wrap_u(u_ode, system, semi)

# Iterate over each particle in the specified fluid system
for particle_idx in TrixiParticles.eachparticle(system)
pc = TrixiParticles.current_coords(u, system, particle_idx)

# Get coordinates for the current particle from the 1D vector
px = pc[1] # x-coordinate
py = pc[2] # y-coordinate

# Check if the particle is inside the sensor's rectangular region
if (x_min <= px <= x_max) && (y_min <= py <= y_max)
# Add its pressure to the sum and increment the count
sum_of_pressures += TrixiParticles.current_pressure(v, system, particle_idx)
num_particles_in_region += 1
end
end

# If no particles are in the region (e.g., before the water hits the wall),
# the pressure is zero.
if num_particles_in_region == 0
return 0.0
end

# Return the calculated average pressure
return sum_of_pressures / num_particles_in_region
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it makes any sense to include this particle-averaged pressure computation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Than remove it in your PR. I have it in all the reference files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I can also remove it manually

Comment on lines +9 to +11
# use at least 5 interpolation points for low resolution simulations
# otherwise use at least the number of particles present
n_interpolation_points = min(5, Int(ceil(sensor_size / particle_spacing)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? I think 10 should be enough.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can change that in your PR all the files are generated with this setting.

@svchb svchb requested a review from efaulhaber July 16, 2025 16:10
@svchb svchb changed the title Validation dam break fix Fix the validation setup based on Marrone2011 Jul 16, 2025
@svchb svchb mentioned this pull request Aug 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants