Skip to content

Resolved TODO in compare_cavity_external.py #267

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 1 commit into
base: main
Choose a base branch
from
Open

Resolved TODO in compare_cavity_external.py #267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,5 @@ validation/references/
# Example/optimiser output directories (all crates)
outputs/
report/
.venv/
venv/
25 changes: 23 additions & 2 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 @@ -104,8 +105,28 @@ 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

# Interpolate external result onto cfd_python grid
x_ext = ext_solver.x
y_ext = ext_solver.y
x_cfd = cfd_python_result["x"]
y_cfd = cfd_python_result["y"]

u_spline = RectBivariateSpline(y_ext, x_ext, ext_sol["u"])
v_spline = RectBivariateSpline(y_ext, x_ext, ext_sol["v"])
p_spline = RectBivariateSpline(y_ext, x_ext, ext_sol["p"])

ext_sol["u"] = u_spline(y_cfd, x_cfd)
ext_sol["v"] = v_spline(y_cfd, x_cfd)
ext_sol["p"] = p_spline(y_cfd, x_cfd)

# Update centerlines
mid_x_idx = len(x_cfd) // 2
mid_y_idx = len(y_cfd) // 2
ext_sol["u_centerline"] = ext_sol["u"][:, mid_x_idx]
ext_sol["v_centerline"] = ext_sol["v"][mid_y_idx, :]
Comment on lines +123 to +127
Copy link
Contributor

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

Choose a reason for hiding this comment

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

high

While the interpolation logic correctly resamples the external solution fields (u, v, p) onto the CFD grid, the plotting function plot_comparison will fail with a dimension mismatch error. This is because plot_comparison uses the grid coordinates from the ext_solver object (ext_solver.X, ext_solver.Y, ext_solver.x, ext_solver.y), which are not updated after interpolation.

To fix this, you should also update these grid attributes on the ext_solver object to match the CFD grid on which the external solution has been interpolated. This will ensure that subsequent plotting calls use consistent data and grid dimensions.

Suggested change
# Update centerlines
mid_x_idx = len(x_cfd) // 2
mid_y_idx = len(y_cfd) // 2
ext_sol["u_centerline"] = ext_sol["u"][:, mid_x_idx]
ext_sol["v_centerline"] = ext_sol["v"][mid_y_idx, :]
# Update centerlines
mid_x_idx = len(x_cfd) // 2
mid_y_idx = len(y_cfd) // 2
ext_sol["u_centerline"] = ext_sol["u"][:, mid_x_idx]
ext_sol["v_centerline"] = ext_sol["v"][mid_y_idx, :]
# Update grid on external solver object for consistent plotting
ext_solver.x = x_cfd
ext_solver.y = y_cfd
ext_solver.X, ext_solver.Y = np.meshgrid(x_cfd, y_cfd)


print("INFO: Interpolated external result onto cfd_python grid using RectBivariateSpline")
Comment on lines +108 to +129
Copy link

@coderabbitai coderabbitai bot Mar 15, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Shape mismatch bug: plot_comparison will fail when grids differ.

After interpolation, ext_sol["u"], ext_sol["v"], ext_sol["p"] are reshaped to match cfd_python grid dimensions. However, plot_comparison (lines 247-263, 278, 291) still uses ext_solver.X, ext_solver.Y, ext_solver.x, ext_solver.y for plotting these interpolated arrays:

  • Line 247: contourf(ext_solver.X, ext_solver.Y, U_mag_ext) — shape mismatch
  • Line 278: plot(ext_sol["u_centerline"], ext_solver.y) — length mismatch
  • Line 291: plot(ext_solver.x, ext_sol["v_centerline"]) — length mismatch

This will raise a runtime error when the grid sizes differ.

🐛 Proposed fix: pass interpolated coordinates or skip external field plots when interpolated

One approach is to track whether interpolation occurred and use the appropriate coordinates:

     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}")

         # Interpolate external result onto cfd_python grid
         x_ext = ext_solver.x
         y_ext = ext_solver.y
         x_cfd = cfd_python_result["x"]
         y_cfd = cfd_python_result["y"]

         u_spline = RectBivariateSpline(y_ext, x_ext, ext_sol["u"])
         v_spline = RectBivariateSpline(y_ext, x_ext, ext_sol["v"])
         p_spline = RectBivariateSpline(y_ext, x_ext, ext_sol["p"])

         ext_sol["u"] = u_spline(y_cfd, x_cfd)
         ext_sol["v"] = v_spline(y_cfd, x_cfd)
         ext_sol["p"] = p_spline(y_cfd, x_cfd)

         # Update centerlines
         mid_x_idx = len(x_cfd) // 2
         mid_y_idx = len(y_cfd) // 2
         ext_sol["u_centerline"] = ext_sol["u"][:, mid_x_idx]
         ext_sol["v_centerline"] = ext_sol["v"][mid_y_idx, :]
+
+        # Store interpolated grid coordinates for plotting
+        ext_sol["_interpolated"] = True
+        ext_sol["_x"] = x_cfd
+        ext_sol["_y"] = y_cfd
+    else:
+        ext_sol["_interpolated"] = False

         print("INFO: Interpolated external result onto cfd_python grid using RectBivariateSpline")

Then update plot_comparison to use these coordinates when ext_sol.get("_interpolated") is True.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@validation/compare_cavity_external.py` around lines 108 - 129, The
interpolated external fields (ext_sol["u"], ext_sol["v"], ext_sol["p"]) are
reshaped to the CFD grid but plotting in plot_comparison still uses
ext_solver.X/Y and ext_solver.x/y causing shape/length mismatches; mark when
interpolation happens (e.g., set ext_sol["_interpolated"]=True and store
ext_sol["_x"]=x_cfd and ext_sol["_y"]=y_cfd) and update plot_comparison to, when
ext_sol.get("_interpolated") is True, use ext_sol["_x"] and ext_sol["_y"] (or
cfd_python_result["x"]/["y"]) for contourf(U_mag_ext) and for plotting
centerlines (use ext_sol["u_centerline"] vs ext_sol["_y"] and
ext_sol["v_centerline"] vs ext_sol["_x"]), otherwise fall back to ext_solver.X/Y
and ext_solver.x/y or skip plotting external fields if coordinates are
incompatible.


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