Skip to content

Commit 4813369

Browse files
committed
refactor: extract repeated patterns in case_validator and test/cases
case_validator.py: - Add _check_order_fits_grid(): the 3-line m/n/p dimension check was duplicated identically in check_igr, check_weno, check_muscl; now one call - Add _get_recon_type(): the recon_type read + prohibit was duplicated in check_weno, check_muscl, check_weno_simulation, check_muscl_simulation; now one call each - Collapse two named param lists in check_muscl into inline literals test/cases.py: - Add make_3d_box_patches(): the 18-line 3-patch 3D box IC setup was copy-pasted identically in mpi_consistency_tests, restart_roundtrip_tests, and kernel_golden_tests; now one line each lint_docs.py: - Add private helpers to skip set (they call prohibit but are not check_* methods and should not require PHYSICS_DOCS entries)
1 parent 00526a0 commit 4813369

3 files changed

Lines changed: 50 additions & 102 deletions

File tree

toolchain/mfc/case_validator.py

Lines changed: 24 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,21 @@ def _validate_logical(self, key: str):
250250
if val is not None and val not in ("T", "F"):
251251
self.errors.append(f"{key} must be 'T' or 'F', got '{val}'")
252252

253+
def _check_order_fits_grid(self, order: int, param_name: str) -> None:
254+
"""Prohibit reconstruction order that exceeds grid cell count in any active dimension."""
255+
m = self.get("m", 0)
256+
n = self.get("n", 0) or 0
257+
p = self.get("p", 0) or 0
258+
self.prohibit(m + 1 < order, f"m must be at least {param_name} - 1 (= {order - 1})")
259+
self.prohibit(n > 0 and n + 1 < order, f"For 2D: n must be at least {param_name} - 1 (= {order - 1})")
260+
self.prohibit(p > 0 and p + 1 < order, f"For 3D: p must be at least {param_name} - 1 (= {order - 1})")
261+
262+
def _get_recon_type(self) -> int:
263+
"""Return recon_type after validating it is 1 (WENO) or 2 (MUSCL)."""
264+
recon_type = self.get("recon_type", 1)
265+
self.prohibit(recon_type not in [1, 2], "recon_type must be 1 (WENO) or 2 (MUSCL)")
266+
return recon_type
267+
253268
def check_parameter_types(self):
254269
"""Validate parameter types before other checks.
255270
@@ -316,72 +331,44 @@ def check_igr(self):
316331
return
317332

318333
igr_order = self.get("igr_order")
319-
m = self.get("m", 0)
320-
n = self.get("n", 0)
321-
p = self.get("p", 0)
322334
self.prohibit(igr_order not in [None, 3, 5], "igr_order must be 3 or 5")
323335
if igr_order:
324-
self.prohibit(m + 1 < igr_order, f"m must be at least igr_order - 1 (= {igr_order - 1})")
325-
self.prohibit(n is not None and n > 0 and n + 1 < igr_order, f"n must be at least igr_order - 1 (= {igr_order - 1})")
326-
self.prohibit(p is not None and p > 0 and p + 1 < igr_order, f"p must be at least igr_order - 1 (= {igr_order - 1})")
336+
self._check_order_fits_grid(igr_order, "igr_order")
327337

328338
def check_weno(self):
329339
"""Checks constraints regarding WENO order"""
330-
recon_type = self.get("recon_type", 1)
331-
self.prohibit(recon_type not in [1, 2], "recon_type must be 1 (WENO) or 2 (MUSCL)")
332-
333-
# WENO_TYPE = 1
334-
if recon_type != 1:
340+
if self._get_recon_type() != 1:
335341
return
336342

337343
for param in ["muscl_order", "muscl_lim"]:
338344
self.prohibit(self.is_set(param), f"recon_type = 1 (WENO) is not compatible with {param}")
339345

340346
weno_order = self.get("weno_order")
341-
m = self.get("m", 0)
342-
n = self.get("n", 0)
343-
p = self.get("p", 0)
344-
345347
if weno_order is None:
346348
return
347349

348350
self.prohibit(weno_order not in [1, 3, 5, 7], "weno_order must be 1, 3, 5, or 7")
349-
self.prohibit(m + 1 < weno_order, f"m must be at least weno_order - 1 (= {weno_order - 1})")
350-
self.prohibit(n is not None and n > 0 and n + 1 < weno_order, f"For 2D simulation, n must be at least weno_order - 1 (= {weno_order - 1})")
351-
self.prohibit(p is not None and p > 0 and p + 1 < weno_order, f"For 3D simulation, p must be at least weno_order - 1 (= {weno_order - 1})")
351+
self._check_order_fits_grid(weno_order, "weno_order")
352352

353353
def check_muscl(self):
354354
"""Check constraints regarding MUSCL order"""
355-
recon_type = self.get("recon_type", 1)
356-
self.prohibit(recon_type not in [1, 2], "recon_type must be 1 (WENO) or 2 (MUSCL)")
357-
358-
# MUSCL_TYPE = 2
359-
if recon_type != 2:
355+
if self._get_recon_type() != 2:
360356
return
361357

362-
weno_log_params = ["mapped_weno", "wenoz", "teno", "mp_weno", "weno_avg", "null_weights", "weno_Re_flux"]
363-
for param in weno_log_params:
358+
for param in ["mapped_weno", "wenoz", "teno", "mp_weno", "weno_avg", "null_weights", "weno_Re_flux"]:
364359
self.prohibit(self.get(param) == "T", f"recon_type = 2 (MUSCL) is not compatible with {param} = T")
365-
366-
weno_numeric_params = ["wenoz_q", "teno_CT", "weno_eps"]
367-
for param in weno_numeric_params:
360+
for param in ["wenoz_q", "teno_CT", "weno_eps"]:
368361
self.prohibit(self.is_set(param), f"recon_type = 2 (MUSCL) is not compatible with {param}")
369362

370363
weno_order = self.get("weno_order")
371364
self.prohibit(weno_order is not None and weno_order != 0, f"recon_type = 2 (MUSCL) requires weno_order unset or 0, but got {weno_order}")
372365

373366
muscl_order = self.get("muscl_order")
374-
m = self.get("m", 0)
375-
n = self.get("n", 0)
376-
p = self.get("p", 0)
377-
378367
if muscl_order is None:
379368
return
380369

381370
self.prohibit(muscl_order not in [1, 2], "muscl_order must be 1 or 2")
382-
self.prohibit(m + 1 < muscl_order, f"m must be at least muscl_order - 1 (= {muscl_order - 1})")
383-
self.prohibit(n is not None and n > 0 and n + 1 < muscl_order, f"For 2D simulation, n must be at least muscl_order - 1 (= {muscl_order - 1})")
384-
self.prohibit(p is not None and p > 0 and p + 1 < muscl_order, f"For 3D simulation, p must be at least muscl_order - 1 (= {muscl_order - 1})")
371+
self._check_order_fits_grid(muscl_order, "muscl_order")
385372

386373
def check_interface_compression(self):
387374
"""Check constraints regarding interface compression"""
@@ -752,11 +739,7 @@ def check_finite_difference(self):
752739

753740
def check_weno_simulation(self):
754741
"""Checks WENO-specific constraints for simulation"""
755-
recon_type = self.get("recon_type", 1)
756-
self.prohibit(recon_type not in [1, 2], "recon_type must be 1 (WENO) or 2 (MUSCL)")
757-
758-
# WENO_TYPE = 1
759-
if recon_type != 1:
742+
if self._get_recon_type() != 1:
760743
return
761744

762745
weno_order = self.get("weno_order")
@@ -793,11 +776,7 @@ def check_weno_simulation(self):
793776

794777
def check_muscl_simulation(self):
795778
"""Checks MUSCL-specific constraints for simulation"""
796-
recon_type = self.get("recon_type", 1)
797-
self.prohibit(recon_type not in [1, 2], "recon_type must be 1 (WENO) or 2 (MUSCL)")
798-
799-
# MUSCL_TYPE = 2
800-
if recon_type != 2:
779+
if self._get_recon_type() != 2:
801780
return
802781

803782
muscl_order = self.get("muscl_order")

toolchain/mfc/lint_docs.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,9 @@ def check_physics_docs_coverage(repo_root: Path) -> list[str]:
407407
# Methods without PHYSICS_DOCS entries. Add a PHYSICS_DOCS entry (with math,
408408
# references, and explanation) to case_validator.py to remove from this set.
409409
skip = {
410+
# Private helpers — called from check_* methods, not check methods themselves
411+
"_check_order_fits_grid",
412+
"_get_recon_type",
410413
# Structural/mechanical checks (no physics meaning)
411414
"check_parameter_types", # type validation
412415
"check_output_format", # output format selection

toolchain/mfc/test/cases.py

Lines changed: 23 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,26 @@ def _h_sweep(case_path, ndim, cons_vars, extra_args, expected, tol, resolutions,
162162
)
163163

164164

165+
def make_3d_box_patches(
166+
z_centroids=(0.05, 0.45, 0.9),
167+
z_lengths=(0.1, 0.7, 0.2),
168+
geometry=9,
169+
) -> dict:
170+
"""3-patch 3D box IC: uniform xy plane (centroid=0.5, length=1), z spacing given."""
171+
d = {}
172+
for pid in range(1, 4):
173+
d[f"patch_icpp({pid})%geometry"] = geometry
174+
for vel in (1, 2, 3):
175+
d[f"patch_icpp({pid})%vel({vel})"] = 0.0
176+
d[f"patch_icpp({pid})%x_centroid"] = 0.5
177+
d[f"patch_icpp({pid})%length_x"] = 1
178+
d[f"patch_icpp({pid})%y_centroid"] = 0.5
179+
d[f"patch_icpp({pid})%length_y"] = 1
180+
d[f"patch_icpp({pid})%z_centroid"] = z_centroids[pid - 1]
181+
d[f"patch_icpp({pid})%length_z"] = z_lengths[pid - 1]
182+
return d
183+
184+
165185
def get_bc_mods(bc: int, dimInfo):
166186
params = {}
167187
for dimCmp in dimInfo[0]:
@@ -2027,25 +2047,7 @@ def mpi_consistency_tests():
20272047
"bc_z%end": -3,
20282048
}
20292049

2030-
for patchID in range(1, 4):
2031-
base_3d[f"patch_icpp({patchID})%geometry"] = 9
2032-
base_3d[f"patch_icpp({patchID})%vel(1)"] = 0.0
2033-
base_3d[f"patch_icpp({patchID})%vel(2)"] = 0.0
2034-
base_3d[f"patch_icpp({patchID})%vel(3)"] = 0.0
2035-
base_3d[f"patch_icpp({patchID})%x_centroid"] = 0.5
2036-
base_3d[f"patch_icpp({patchID})%length_x"] = 1
2037-
base_3d[f"patch_icpp({patchID})%y_centroid"] = 0.5
2038-
base_3d[f"patch_icpp({patchID})%length_y"] = 1
2039-
base_3d.update(
2040-
{
2041-
"patch_icpp(1)%z_centroid": 0.05,
2042-
"patch_icpp(1)%length_z": 0.1,
2043-
"patch_icpp(2)%z_centroid": 0.45,
2044-
"patch_icpp(2)%length_z": 0.7,
2045-
"patch_icpp(3)%z_centroid": 0.9,
2046-
"patch_icpp(3)%length_z": 0.2,
2047-
}
2048-
)
2050+
base_3d.update(make_3d_box_patches())
20492051

20502052
# Bubbles with 2 MPI ranks
20512053
stack.push(
@@ -2203,25 +2205,7 @@ def restart_roundtrip_tests():
22032205
"bc_z%beg": -3,
22042206
"bc_z%end": -3,
22052207
}
2206-
for patchID in range(1, 4):
2207-
base_3d[f"patch_icpp({patchID})%geometry"] = 9
2208-
base_3d[f"patch_icpp({patchID})%vel(1)"] = 0.0
2209-
base_3d[f"patch_icpp({patchID})%vel(2)"] = 0.0
2210-
base_3d[f"patch_icpp({patchID})%vel(3)"] = 0.0
2211-
base_3d[f"patch_icpp({patchID})%x_centroid"] = 0.5
2212-
base_3d[f"patch_icpp({patchID})%length_x"] = 1
2213-
base_3d[f"patch_icpp({patchID})%y_centroid"] = 0.5
2214-
base_3d[f"patch_icpp({patchID})%length_y"] = 1
2215-
base_3d.update(
2216-
{
2217-
"patch_icpp(1)%z_centroid": 0.05,
2218-
"patch_icpp(1)%length_z": 0.1,
2219-
"patch_icpp(2)%z_centroid": 0.45,
2220-
"patch_icpp(2)%length_z": 0.7,
2221-
"patch_icpp(3)%z_centroid": 0.9,
2222-
"patch_icpp(3)%length_z": 0.2,
2223-
}
2224-
)
2208+
base_3d.update(make_3d_box_patches())
22252209
stack.push("Restart Roundtrip -> 3D", base_3d)
22262210
cases.append(define_case_d(stack, "", {}, restart_check=True))
22272211
stack.pop()
@@ -2259,25 +2243,7 @@ def kernel_golden_tests():
22592243
"bc_z%beg": -3,
22602244
"bc_z%end": -3,
22612245
}
2262-
for patchID in range(1, 4):
2263-
base_3d[f"patch_icpp({patchID})%geometry"] = 9
2264-
base_3d[f"patch_icpp({patchID})%vel(1)"] = 0.0
2265-
base_3d[f"patch_icpp({patchID})%vel(2)"] = 0.0
2266-
base_3d[f"patch_icpp({patchID})%vel(3)"] = 0.0
2267-
base_3d[f"patch_icpp({patchID})%x_centroid"] = 0.5
2268-
base_3d[f"patch_icpp({patchID})%length_x"] = 1
2269-
base_3d[f"patch_icpp({patchID})%y_centroid"] = 0.5
2270-
base_3d[f"patch_icpp({patchID})%length_y"] = 1
2271-
base_3d.update(
2272-
{
2273-
"patch_icpp(1)%z_centroid": 0.05,
2274-
"patch_icpp(1)%length_z": 0.1,
2275-
"patch_icpp(2)%z_centroid": 0.45,
2276-
"patch_icpp(2)%length_z": 0.7,
2277-
"patch_icpp(3)%z_centroid": 0.9,
2278-
"patch_icpp(3)%length_z": 0.2,
2279-
}
2280-
)
2246+
base_3d.update(make_3d_box_patches())
22812247

22822248
# 3D grid stretching in all directions.
22832249
# The cosh-based stretching expands the domain beyond the original

0 commit comments

Comments
 (0)