Skip to content

Implement CFD grid interpolation logic for validation #268

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.

Sign up for GitHub

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
ryancinsight wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Implement CFD grid interpolation logic for validation #268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5b674c7
Implement TODO: Interpolate CFD grid data if shape differs from exter…
google-labs-jules[bot] Mar 16, 2026
70fffcc
Fix Performance Benchmarks CI missing bench target
google-labs-jules[bot] Mar 16, 2026
bda3362
Fix Performance Benchmarks CI missing fontconfig dependency
google-labs-jules[bot] Mar 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/performance-benchmarking.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,16 @@ jobs:
run: |
if [ "$RUNNER_OS" == "Linux" ]; then
sudo apt-get update
sudo apt-get install -y valgrind linux-tools-common
sudo apt-get install -y valgrind linux-tools-common libfontconfig1-dev
fi

- name: Build benchmarks
run: cargo build --release --bench comprehensive_cfd_benchmarks
run: cargo build --release --bench performance_benchmarks

- name: Run comprehensive benchmarks
if: github.event.inputs.benchmark_type == 'comprehensive' || github.event_name != 'workflow_dispatch'
run: |
cargo bench --bench comprehensive_cfd_benchmarks | tee benchmark_results.txt
cargo bench --bench performance_benchmarks | tee benchmark_results.txt

- name: Run regression detection benchmarks
if: github.event.inputs.benchmark_type == 'regression' || github.event_name == 'schedule'
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,5 @@ report/
*.dll
*.so
*.dylib
venv/
__pycache__/
62 changes: 44 additions & 18 deletions validation/compare_cavity_external.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import numpy as np
import matplotlib.pyplot as plt
from pathlib import Path
from scipy.interpolate import RectBivariateSpline

# Add validation directory to path
sys.path.insert(0, str(Path(__file__).parent))
Expand Down Expand Up @@ -49,24 +50,26 @@ def run_cfd_python_cavity(Re: float = 100, nx: int = 65, ny: int = 65):
if hasattr(cfd_python, 'CavitySolver2D'):
solver = cfd_python.CavitySolver2D(
nx=nx, ny=ny,
Re=Re,
tolerance=1e-6,
max_iterations=20000
reynolds=Re
)
result = solver.solve()

return {
"u": np.array(result.u_field),
"v": np.array(result.v_field),
"p": np.array(result.p_field),
"u_centerline": np.array(result.u_centerline),
"v_centerline": np.array(result.v_centerline),
"x": np.array(result.x_coords),
"y": np.array(result.y_coords),
"converged": result.converged,
"iterations": result.iterations,
"residual": result.residual
}
if hasattr(result, "u_field"):
return {
"u": np.array(result.u_field),
"v": np.array(result.v_field),
"p": np.array(result.p_field),
Comment on lines +57 to +61
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 16, 2026

Choose a reason for hiding this comment

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

high

The check for u_field improves robustness. However, it implicitly assumes that if u_field exists, v_field and p_field also exist. This could lead to an AttributeError if one of them is missing. It would be safer to explicitly check for all required fields before attempting to access them.

Suggested change
if hasattr(result, "u_field"):
return {
"u": np.array(result.u_field),
"v": np.array(result.v_field),
"p": np.array(result.p_field),
if all(hasattr(result, f) for f in ("u_field", "v_field", "p_field")):
return {
"u": np.array(result.u_field),
"v": np.array(result.v_field),
"p": np.array(result.p_field),

"u_centerline": np.array(result.u_centerline),
"v_centerline": np.array(result.v_centerline),
"x": np.array(result.x_coords),
"y": np.array(result.y_coords),
"converged": result.converged,
"iterations": getattr(result, "iterations", 0),
"residual": getattr(result, "residual", 0.0)
}
else:
print("WARN: cfd_python cavity solver does not export full fields, skipping 2D comparison.")
return None
else:
print("WARN: CavitySolver2D not found in cfd_python - using placeholder")
# Return dummy data matching external reference dimensions
Expand Down Expand Up @@ -103,9 +106,32 @@ def compare_solutions(cfd_python_result, external_result, Re: float):

# Ensure same grid size
if cfd_python_result["u"].shape != ext_sol["u"].shape:
print(f"WARN: Grid size mismatch: cfd_python {cfd_python_result['u'].shape} vs external {ext_sol['u'].shape}")
# TODO: Interpolate if needed
return None
print(f"INFO: Interpolating cfd_python results from {cfd_python_result['u'].shape} to external {ext_sol['u'].shape} grid")

# Original grid (assuming regular grid)
# Note: RectBivariateSpline expects strictly increasing x and y
x_orig = cfd_python_result["x"]
y_orig = cfd_python_result["y"]

# Target grid
x_new = ext_sol["x"]
y_new = ext_sol["y"]

# Interpolate fields (transpose as RectBivariateSpline expects [x, y] ordering)
spline_u = RectBivariateSpline(x_orig, y_orig, cfd_python_result["u"].T)
Copy link

@recurseml recurseml bot Mar 16, 2026

Choose a reason for hiding this comment

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

RectBivariateSpline requires coordinate arrays to be strictly ascending. The code does not validate that x_orig and y_orig (from cfd_python result.x_coords and result.y_coords) are strictly ascending before calling RectBivariateSpline. If the Rust solver returns non-ascending, descending, or duplicate coordinate values, this will crash with a ValueError. According to scipy documentation: 'x,y array_like: 1-D arrays of coordinates in strictly ascending order.' The same issue affects lines 122 and 123. This validation script will crash at runtime if grid coordinates from cfd_python are not properly sorted.

React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

spline_v = RectBivariateSpline(x_orig, y_orig, cfd_python_result["v"].T)
spline_p = RectBivariateSpline(x_orig, y_orig, cfd_python_result["p"].T)

# Evaluate on new grid (and transpose back to [y, x] ordering)
cfd_python_result["u"] = spline_u(x_new, y_new).T
cfd_python_result["v"] = spline_v(x_new, y_new).T
cfd_python_result["p"] = spline_p(x_new, y_new).T
Comment on lines +120 to +128
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 16, 2026

Choose a reason for hiding this comment

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

medium

To improve maintainability and reduce code duplication, the interpolation logic for the u, v, and p fields can be refactored into a loop. This makes the code more concise and easier to modify if more fields need interpolation in the future.

Suggested change
# Interpolate fields (transpose as RectBivariateSpline expects [x, y] ordering)
spline_u = RectBivariateSpline(x_orig, y_orig, cfd_python_result["u"].T)
spline_v = RectBivariateSpline(x_orig, y_orig, cfd_python_result["v"].T)
spline_p = RectBivariateSpline(x_orig, y_orig, cfd_python_result["p"].T)
# Evaluate on new grid (and transpose back to [y, x] ordering)
cfd_python_result["u"] = spline_u(x_new, y_new).T
cfd_python_result["v"] = spline_v(x_new, y_new).T
cfd_python_result["p"] = spline_p(x_new, y_new).T
# Interpolate fields (transpose as RectBivariateSpline expects [x, y] ordering)
for field_key in ("u", "v", "p"):
spline = RectBivariateSpline(x_orig, y_orig, cfd_python_result[field_key].T)
# Evaluate on new grid and transpose back to [y, x] ordering
cfd_python_result[field_key] = spline(x_new, y_new).T


# Update centerlines and coordinates
cfd_python_result["u_centerline"] = cfd_python_result["u"][:, len(x_new) // 2]
cfd_python_result["v_centerline"] = cfd_python_result["v"][len(y_new) // 2, :]
cfd_python_result["x"] = x_new
cfd_python_result["y"] = y_new

# Compute L2 errors
u_diff = cfd_python_result["u"] - ext_sol["u"]
Expand Down
Loading