Skip to content

Commit 00526a0

Browse files
committed
refactor: fix four accumulated code quality issues
- run/run.py: replace 4 duplicate profiler if-blocks with data-driven table; adding a new profiler now requires one entry in _PROFILERS - lint_source.py: remove check_pylint_directives — MFC uses ruff, no pylint directives exist anywhere in the codebase, check was provably dead code - bench.py: delete self-deprecating TODO comment - run/input.py: log Cantera candidate failures instead of swallowing silently; previously a Cantera yaml parse error looked identical to file-not-found
1 parent af8dc52 commit 00526a0

6 files changed

Lines changed: 257 additions & 118 deletions

File tree

toolchain/mfc/bench.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,6 @@ def bench(targets=None):
166166
cons.unindent()
167167

168168

169-
# TODO: This function is too long and not nicely written at all. Someone should
170-
# refactor it...
171169
def diff():
172170
lhs, rhs = file_load_yaml(ARG("lhs")), file_load_yaml(ARG("rhs"))
173171
lhs_path = os.path.relpath(ARG("lhs"))

toolchain/mfc/lint_source.py

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -312,33 +312,6 @@ def check_junk_comments(repo_root: Path) -> list[str]:
312312
return errors
313313

314314

315-
def check_pylint_directives(repo_root: Path) -> list[str]:
316-
"""Flag ``# pylint:`` directives in Python files.
317-
318-
MFC uses ruff for linting; leftover pylint directives are dead code.
319-
"""
320-
errors: list[str] = []
321-
pylint_re = re.compile(r"#\s*pylint\s*:", re.IGNORECASE)
322-
self_path = Path(__file__).resolve()
323-
324-
for subdir in ["examples", "benchmarks", "toolchain"]:
325-
d = repo_root / subdir
326-
if not d.exists():
327-
continue
328-
for py in sorted(d.rglob("*.py")):
329-
if py.resolve() == self_path:
330-
continue
331-
lines = py.read_text(encoding="utf-8").splitlines()
332-
rel = py.relative_to(repo_root)
333-
334-
for i, line in enumerate(lines):
335-
match = pylint_re.search(line)
336-
if match:
337-
errors.append(f" {rel}:{i + 1} pylint directive. Fix: remove (use ruff noqa comments if needed)")
338-
339-
return errors
340-
341-
342315
def main():
343316
repo_root = Path(__file__).resolve().parents[2]
344317

@@ -351,7 +324,6 @@ def main():
351324
all_errors.extend(check_fypp_list_duplicates(repo_root))
352325
all_errors.extend(check_duplicate_lines(repo_root))
353326
all_errors.extend(check_hardcoded_byte_size(repo_root))
354-
all_errors.extend(check_pylint_directives(repo_root))
355327

356328
if all_errors:
357329
print("Source lint failed:")

toolchain/mfc/params/definitions.py

Lines changed: 238 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ def _fc(name: str, default: int) -> int:
3939
NA = 4 # acoustic sources: enumerated individually
4040

4141

42-
4342
# Parameters that can be hard-coded for GPU case optimization
4443
CASE_OPT_PARAMS = {
4544
"mapped_weno",
@@ -496,6 +495,7 @@ def _r(name, ptype, tags=None, desc=None, hint=None, math=None, str_len=None):
496495
hint = _lookup_hint(name)
497496
if desc is None:
498497
from .descriptions import get_description
498+
499499
desc = get_description(name)
500500
constraint = CONSTRAINTS.get(name)
501501
if constraint and "value_labels" in constraint:
@@ -1028,78 +1028,259 @@ def _init_registry():
10281028

10291029
NAMELIST_VARS: dict[str, set[str]] = {}
10301030

1031+
10311032
def _nv(targets: set, *names: str) -> None:
10321033
for n in names:
10331034
NAMELIST_VARS[n] = set(targets)
10341035

1036+
10351037
_ALL, _PRE_SIM, _SIM_POST = {"pre", "sim", "post"}, {"pre", "sim"}, {"sim", "post"}
10361038
_PRE_POST = {"pre", "post"}
10371039
_SIM = {"sim"}
10381040
_PRE = {"pre"}
10391041
_POST = {"post"}
10401042

1041-
_nv(_ALL,
1042-
"m", "n", "p", "cyl_coord", "bc_x", "bc_y", "bc_z", "num_bc_patches", "case_dir",
1043-
"t_step_start", "cfl_adap_dt", "cfl_const_dt", "n_start",
1044-
"model_eqns", "mpp_lim", "relax", "relax_model",
1045-
"fluid_pp", "bub_pp", "rhoref", "pref",
1046-
"bubbles_euler", "bubbles_lagrange", "R0ref", "polytropic", "thermal",
1047-
"Ca", "Web", "Re_inv", "polydisperse", "poly_sigma", "qbmm", "sigma", "adv_n",
1048-
"hypoelasticity", "hyperelasticity", "surface_tension", "relativity",
1049-
"ib", "num_ibs", "cont_damage", "hyper_cleaning", "Bx0",
1050-
"precision", "parallel_io", "file_per_process", "fft_wrt", "down_sample",
1043+
_nv(
1044+
_ALL,
1045+
"m",
1046+
"n",
1047+
"p",
1048+
"cyl_coord",
1049+
"bc_x",
1050+
"bc_y",
1051+
"bc_z",
1052+
"num_bc_patches",
1053+
"case_dir",
1054+
"t_step_start",
1055+
"cfl_adap_dt",
1056+
"cfl_const_dt",
1057+
"n_start",
1058+
"model_eqns",
1059+
"mpp_lim",
1060+
"relax",
1061+
"relax_model",
1062+
"fluid_pp",
1063+
"bub_pp",
1064+
"rhoref",
1065+
"pref",
1066+
"bubbles_euler",
1067+
"bubbles_lagrange",
1068+
"R0ref",
1069+
"polytropic",
1070+
"thermal",
1071+
"Ca",
1072+
"Web",
1073+
"Re_inv",
1074+
"polydisperse",
1075+
"poly_sigma",
1076+
"qbmm",
1077+
"sigma",
1078+
"adv_n",
1079+
"hypoelasticity",
1080+
"hyperelasticity",
1081+
"surface_tension",
1082+
"relativity",
1083+
"ib",
1084+
"num_ibs",
1085+
"cont_damage",
1086+
"hyper_cleaning",
1087+
"Bx0",
1088+
"precision",
1089+
"parallel_io",
1090+
"file_per_process",
1091+
"fft_wrt",
1092+
"down_sample",
10511093
)
1052-
_nv(_SIM_POST,
1053-
"t_step_stop", "t_step_save", "t_stop", "t_save", "cfl_target",
1054-
"avg_state", "prim_vars_wrt", "alt_soundspeed", "mixture_err", "fd_order", "ib_state_wrt",
1094+
_nv(
1095+
_SIM_POST,
1096+
"t_step_stop",
1097+
"t_step_save",
1098+
"t_stop",
1099+
"t_save",
1100+
"cfl_target",
1101+
"avg_state",
1102+
"prim_vars_wrt",
1103+
"alt_soundspeed",
1104+
"mixture_err",
1105+
"fd_order",
1106+
"ib_state_wrt",
10551107
)
1056-
_nv(_PRE_SIM,
1057-
"x_domain", "y_domain", "z_domain",
1058-
"x_a", "y_a", "z_a", "x_b", "y_b", "z_b",
1059-
"palpha_eps", "ptgalpha_eps", "t_step_old", "patch_ib", "pi_fac",
1108+
_nv(
1109+
_PRE_SIM,
1110+
"x_domain",
1111+
"y_domain",
1112+
"z_domain",
1113+
"x_a",
1114+
"y_a",
1115+
"z_a",
1116+
"x_b",
1117+
"y_b",
1118+
"z_b",
1119+
"palpha_eps",
1120+
"ptgalpha_eps",
1121+
"t_step_old",
1122+
"patch_ib",
1123+
"pi_fac",
10601124
)
10611125
_nv(_PRE_POST, "num_fluids", "weno_order", "recon_type", "muscl_order", "mhd", "nb", "sigR", "igr", "igr_order")
1062-
_nv(_SIM,
1063-
"dt", "t_step_print", "time_stepper", "adap_dt", "adap_dt_tol", "adap_dt_max_iters",
1064-
"weno_eps", "teno_CT", "wenoz_q", "mp_weno", "weno_avg", "weno_Re_flux", "null_weights",
1065-
"muscl_eps", "int_comp", "ic_eps", "ic_beta",
1066-
"riemann_solver", "wave_speeds", "low_Mach",
1067-
"hyper_cleaning_speed", "hyper_cleaning_tau",
1068-
"run_time_info", "bubble_model", "lag_params",
1069-
"probe_wrt", "num_probes", "probe", "integral_wrt", "num_integrals", "integral",
1070-
"acoustic_source", "num_source", "acoustic", "chem_params",
1071-
"bf_x", "bf_y", "bf_z", "k_x", "k_y", "k_z", "w_x", "w_y", "w_z",
1072-
"p_x", "p_y", "p_z", "g_x", "g_y", "g_z",
1073-
"collision_model", "coefficient_of_restitution", "collision_time", "ib_coefficient_of_friction",
1074-
"tau_star", "cont_damage_s", "alpha_bar",
1075-
"rdma_mpi", "alf_factor",
1076-
"num_igr_iters", "num_igr_warm_start_iters", "igr_iter_solver", "igr_pres_lim",
1077-
"nv_uvm_out_of_core", "nv_uvm_igr_temps_on_gpu", "nv_uvm_pref_gpu",
1126+
_nv(
1127+
_SIM,
1128+
"dt",
1129+
"t_step_print",
1130+
"time_stepper",
1131+
"adap_dt",
1132+
"adap_dt_tol",
1133+
"adap_dt_max_iters",
1134+
"weno_eps",
1135+
"teno_CT",
1136+
"wenoz_q",
1137+
"mp_weno",
1138+
"weno_avg",
1139+
"weno_Re_flux",
1140+
"null_weights",
1141+
"muscl_eps",
1142+
"int_comp",
1143+
"ic_eps",
1144+
"ic_beta",
1145+
"riemann_solver",
1146+
"wave_speeds",
1147+
"low_Mach",
1148+
"hyper_cleaning_speed",
1149+
"hyper_cleaning_tau",
1150+
"run_time_info",
1151+
"bubble_model",
1152+
"lag_params",
1153+
"probe_wrt",
1154+
"num_probes",
1155+
"probe",
1156+
"integral_wrt",
1157+
"num_integrals",
1158+
"integral",
1159+
"acoustic_source",
1160+
"num_source",
1161+
"acoustic",
1162+
"chem_params",
1163+
"bf_x",
1164+
"bf_y",
1165+
"bf_z",
1166+
"k_x",
1167+
"k_y",
1168+
"k_z",
1169+
"w_x",
1170+
"w_y",
1171+
"w_z",
1172+
"p_x",
1173+
"p_y",
1174+
"p_z",
1175+
"g_x",
1176+
"g_y",
1177+
"g_z",
1178+
"collision_model",
1179+
"coefficient_of_restitution",
1180+
"collision_time",
1181+
"ib_coefficient_of_friction",
1182+
"tau_star",
1183+
"cont_damage_s",
1184+
"alpha_bar",
1185+
"rdma_mpi",
1186+
"alf_factor",
1187+
"num_igr_iters",
1188+
"num_igr_warm_start_iters",
1189+
"igr_iter_solver",
1190+
"igr_pres_lim",
1191+
"nv_uvm_out_of_core",
1192+
"nv_uvm_igr_temps_on_gpu",
1193+
"nv_uvm_pref_gpu",
10781194
)
1079-
_nv(_PRE,
1080-
"stretch_x", "stretch_y", "stretch_z", "a_x", "a_y", "a_z", "loops_x", "loops_y", "loops_z",
1081-
"n_start_old", "num_patches", "patch_icpp", "patch_bc",
1082-
"sigV", "dist_type", "rhoRV", "viscous",
1083-
"old_grid", "old_ic",
1084-
"perturb_flow", "perturb_flow_fluid", "perturb_flow_mag",
1085-
"perturb_sph", "perturb_sph_fluid", "fluid_rho",
1086-
"mixlayer_vel_profile", "mixlayer_vel_coef", "mixlayer_perturb",
1087-
"mixlayer_perturb_nk", "mixlayer_perturb_k0",
1088-
"pre_stress", "elliptic_smoothing", "elliptic_smoothing_iters",
1089-
"simplex_perturb", "simplex_params",
1195+
_nv(
1196+
_PRE,
1197+
"stretch_x",
1198+
"stretch_y",
1199+
"stretch_z",
1200+
"a_x",
1201+
"a_y",
1202+
"a_z",
1203+
"loops_x",
1204+
"loops_y",
1205+
"loops_z",
1206+
"n_start_old",
1207+
"num_patches",
1208+
"patch_icpp",
1209+
"patch_bc",
1210+
"sigV",
1211+
"dist_type",
1212+
"rhoRV",
1213+
"viscous",
1214+
"old_grid",
1215+
"old_ic",
1216+
"perturb_flow",
1217+
"perturb_flow_fluid",
1218+
"perturb_flow_mag",
1219+
"perturb_sph",
1220+
"perturb_sph_fluid",
1221+
"fluid_rho",
1222+
"mixlayer_vel_profile",
1223+
"mixlayer_vel_coef",
1224+
"mixlayer_perturb",
1225+
"mixlayer_perturb_nk",
1226+
"mixlayer_perturb_k0",
1227+
"pre_stress",
1228+
"elliptic_smoothing",
1229+
"elliptic_smoothing_iters",
1230+
"simplex_perturb",
1231+
"simplex_params",
10901232
)
1091-
_nv(_POST,
1092-
"x_output", "y_output", "z_output",
1093-
"format", "output_partial_domain", "sim_data", "G", "flux_lim", "cons_vars_wrt",
1094-
"rho_wrt", "E_wrt", "pres_wrt", "c_wrt",
1095-
"gamma_wrt", "heat_ratio_wrt", "pi_inf_wrt", "pres_inf_wrt",
1096-
"omega_wrt", "qm_wrt", "liutex_wrt", "schlieren_wrt", "schlieren_alpha",
1097-
"alpha_rho_wrt", "mom_wrt", "vel_wrt", "flux_wrt", "alpha_wrt", "cf_wrt",
1098-
"chem_wrt_T", "chem_wrt_Y", "alpha_rho_e_wrt",
1099-
"lag_header", "lag_txt_wrt", "lag_db_wrt", "lag_id_wrt",
1100-
"lag_pos_wrt", "lag_pos_prev_wrt", "lag_vel_wrt", "lag_rad_wrt", "lag_rvel_wrt",
1101-
"lag_r0_wrt", "lag_rmax_wrt", "lag_rmin_wrt", "lag_dphidt_wrt",
1102-
"lag_pres_wrt", "lag_mv_wrt", "lag_mg_wrt", "lag_betaT_wrt", "lag_betaC_wrt",
1233+
_nv(
1234+
_POST,
1235+
"x_output",
1236+
"y_output",
1237+
"z_output",
1238+
"format",
1239+
"output_partial_domain",
1240+
"sim_data",
1241+
"G",
1242+
"flux_lim",
1243+
"cons_vars_wrt",
1244+
"rho_wrt",
1245+
"E_wrt",
1246+
"pres_wrt",
1247+
"c_wrt",
1248+
"gamma_wrt",
1249+
"heat_ratio_wrt",
1250+
"pi_inf_wrt",
1251+
"pres_inf_wrt",
1252+
"omega_wrt",
1253+
"qm_wrt",
1254+
"liutex_wrt",
1255+
"schlieren_wrt",
1256+
"schlieren_alpha",
1257+
"alpha_rho_wrt",
1258+
"mom_wrt",
1259+
"vel_wrt",
1260+
"flux_wrt",
1261+
"alpha_wrt",
1262+
"cf_wrt",
1263+
"chem_wrt_T",
1264+
"chem_wrt_Y",
1265+
"alpha_rho_e_wrt",
1266+
"lag_header",
1267+
"lag_txt_wrt",
1268+
"lag_db_wrt",
1269+
"lag_id_wrt",
1270+
"lag_pos_wrt",
1271+
"lag_pos_prev_wrt",
1272+
"lag_vel_wrt",
1273+
"lag_rad_wrt",
1274+
"lag_rvel_wrt",
1275+
"lag_r0_wrt",
1276+
"lag_rmax_wrt",
1277+
"lag_rmin_wrt",
1278+
"lag_dphidt_wrt",
1279+
"lag_pres_wrt",
1280+
"lag_mv_wrt",
1281+
"lag_mg_wrt",
1282+
"lag_betaT_wrt",
1283+
"lag_betaC_wrt",
11031284
)
11041285

11051286
# Case-optimization params appear in the sim namelist under #:if not MFC_CASE_OPTIMIZATION.

0 commit comments

Comments
 (0)