add core changes to probe gen #1052
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -168,9 +168,7 @@ contains | |
| $:GPU_UPDATE(host='[accel_mag]') | ||
|
|
||
| call s_derive_center_of_mass(q_prim_ts2(2)%vf, c_mass) | ||
| call s_write_probe_files(t_step, q_cons_ts(1)%vf, accel_mag) | ||
| call s_write_com_files(t_step, c_mass) | ||
| end if | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -136,6 +136,10 @@ def _extract_doubles(s: str) -> list: | |
| lines = content.splitlines() | ||
| content = "\n".join(lines[1:]) # Skip the first line | ||
| doubles = _extract_doubles(content) | ||
| elif 'probe' in short_filepath: | ||
| # Probes are constructed as <time> <value1> <value2> ... | ||
| # Just discard the first column | ||
| doubles = [float(e) for line in content.strip().splitlines() if line.strip() for e in _extract_doubles(line)[1:]] | ||
| else: | ||
| # Every line is <x> <y> <z> <value> (<y> and <z> are optional). So the | ||
| # number of dimensions is the number of doubles in the first line minus 1. | ||
|
|
||
|
Contributor
There was a problem hiding this comment. High-level SuggestionThe automatic probe placement at 1/3 and 2/3 of the domain is a generic heuristic that might miss key physical phenomena at boundaries or interfaces. Consider making this feature opt-in or providing easier override mechanisms to ensure probes monitor relevant regions. [High-level, importance: 8] Solution Walkthrough:Before:# In toolchain/mfc/test/case.py
# ...
# This logic is applied to all test cases by default (opt-out)
if 'probe_wrt' not in case:
mods['probe_wrt'] = 'T'
if case_is_1D:
mods['num_probes'] = 2
# Place 2 probes at 1/3 and 2/3 of the domain
elif case_is_2D:
mods['num_probes'] = 4
# Place 4 probes in a 2x2 grid at 1/3 and 2/3 of domain
else: # 3D
mods['num_probes'] = 8
# Place 8 probes in a 2x2x2 grid at 1/3 and 2/3 of domainAfter:# In toolchain/mfc/test/case.py
# ...
# Suggestion: Make this an opt-in feature
if case.get('auto_probes', False):
mods['probe_wrt'] = 'T'
if case_is_1D:
mods['num_probes'] = 2
# Place 2 probes at 1/3 and 2/3 of the domain
elif case_is_2D:
mods['num_probes'] = 4
# Place 4 probes in a 2x2 grid at 1/3 and 2/3 of domain
else: # 3D
mods['num_probes'] = 8
# Place 8 probes in a 2x2x2 grid at 1/3 and 2/3 of domain
# Individual tests would need to set `auto_probes: True` to enable this. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -230,8 +230,36 @@ def create_directory(self): | |
| mods['omega_wrt(2)'] = 'T' | ||
| mods['omega_wrt(3)'] = 'T' | ||
| else: | ||
| mods['parallel_io'] = 'F' | ||
| mods['prim_vars_wrt'] = 'F' | ||
| if ('fd_order' not in case) and ('fd_order' not in mods): | ||
| mods['fd_order'] = 1 | ||
| if ('probe_wrt' not in case): | ||
| mods['probe_wrt'] = 'T' | ||
| if case['n'] == 0: | ||
| mods['num_probes'] = 2 | ||
| dx = (case['x_domain%end'] - case['x_domain%beg']) / 3 | ||
| for i in range(2): | ||
| mods[f'probe({{i+1}})%x'] = case['x_domain%beg'] + (i+1) * dx | ||
| elif case['p'] == 0: | ||
| mods['num_probes'] = 4 | ||
| dx = (case['x_domain%end'] - case['x_domain%beg']) / 3 | ||
| dy = (case['y_domain%end'] - case['y_domain%beg']) / 3 | ||
| for i in range(2): | ||
| for j in range(2): | ||
| mods[f'probe({{i*2+j+1}})%x'] = case['x_domain%beg'] + (i+1) * dx | ||
| mods[f'probe({{i*2+j+1}})%y'] = case['y_domain%beg'] + (j+1) * dy | ||
| else: | ||
| mods['num_probes'] = 8 | ||
| dx = (case['x_domain%end'] - case['x_domain%beg']) / 3 | ||
| dy = (case['y_domain%end'] - case['y_domain%beg']) / 3 | ||
| dz = (case['z_domain%end'] - case['z_domain%beg']) / 3 | ||
| for i in range(2): | ||
| for j in range(2): | ||
| for k in range(2): | ||
| mods[f'probe({{i*4+j*2+k+1}})%x'] = case['x_domain%beg'] + (i+1) * dx | ||
| mods[f'probe({{i*4+j*2+k+1}})%y'] = case['y_domain%beg'] + (j+1) * dy | ||
| mods[f'probe({{i*4+j*2+k+1}})%z'] = case['z_domain%beg'] + (k+1) * dz | ||
|
|
||
|
Comment on lines
+233
to
263
Contributor
There was a problem hiding this comment. 🧩 Analysis chainAuto probe defaults are reasonable but rely on domain keys being present The new else-branch behavior does a few important things for tests:
Two things to double‑check:
Overall the structure is sound; just ensure the domain-key assumption holds across the full test suite. I'll verify the concerns raised in the review comment by examining the context around the code snippet and checking for domain key initialization patterns. Length of output: 4017 🏁 Script executed: #!/bin/bash
# Search for domain key initialization patterns in case.py
rg -n "x_domain|y_domain|z_domain" toolchain/mfc/test/case.py -B2 -A2 | head -50Length of output: 1453 🏁 Script executed: #!/bin/bash
# Look for any guards or error handling around domain key access
rg -n "KeyError|try:|except|in case" toolchain/mfc/test/case.py | head -30Length of output: 103 🏁 Script executed: #!/bin/bash
# Check the function signature and initial case dict population
rg -n "def |case\s*=" toolchain/mfc/test/case.py | head -20Length of output: 1037 Length of output: 526 🏁 Script executed: #!/bin/bash
# Search for test case definitions and domain key initialization
rg -n "x_domain|domain" toolchain/mfc/test/ -l | head -10Length of output: 54 🏁 Script executed: #!/bin/bash
# Look for where TestCase instances are created with mods
rg -n "define_case_f|mods\s*=" toolchain/mfc/test/ --type=py -B2 -A5 | head -60Length of output: 3759 🏁 Script executed: #!/bin/bash
# Check if there are any test cases in the test directory
find toolchain/mfc/test -name "*.py" -type f | grep -v __pycache__Length of output: 112 Length of output: 523 🏁 Script executed: #!/bin/bash
# Look for actual test case definitions with domain info in cases.py
rg -n "x_domain|y_domain|z_domain|define_case" toolchain/mfc/test/cases.py -B2 -A2 | head -100Length of output: 5227 🏁 Script executed: #!/bin/bash
# Search for examples dir to see actual case.py files that might lack domain keys
find examples -name "case.py" -type f 2>/dev/null | head -5Length of output: 209 🏁 Script executed: #!/bin/bash
# Examine one example case file to see domain key patterns
find examples -name "case.py" -type f 2>/dev/null | head -1 | xargs cat | head -80Length of output: 1605 Length of output: 700 🏁 Script executed: #!/bin/bash
# Look for TestCase class definition and __init__ method
rg -n "class TestCase|def __init__" toolchain/mfc/test/case.py -A12 | head -60Length of output: 1534 🏁 Script executed: #!/bin/bash
# Check a few more example case files to see if they all have domain keys
find examples -name "case.py" -type f 2>/dev/null | head -3 | xargs -I {} sh -c 'echo "=== {} ===" && grep -E "x_domain|y_domain|z_domain" {} || echo "NO DOMAIN KEYS"'Length of output: 351 🏁 Script executed: #!/bin/bash
# Look at cases.py to see if there's any test that passes custom mods with domain keys
rg -n "x_domain|y_domain|z_domain" toolchain/mfc/test/cases.py -B3 -A3Length of output: 1979 Length of output: 1307 🏁 Script executed: #!/bin/bash
# Look for case.Case import and find where it's defined
rg -n "^import|^from.*case|class Case" toolchain/mfc/test/case.py | head -20Length of output: 181 🏁 Script executed: #!/bin/bash
# Search for BASE_CFG and gen_json_dict_str in the input module (likely case.Case parent)
find toolchain -name "*.py" -type f | xargs rg -l "BASE_CFG|gen_json_dict_str" | head -5Length of output: 48 🏁 Script executed: #!/bin/bash
# Check if any test case in cases.py is defined without domain keys
rg -n "define_case_d|define_case_f" toolchain/mfc/test/cases.py -A3 | grep -v "x_domain\|y_domain\|z_domain" | head -50Length of output: 1874 Length of output: 87 🏁 Script executed: #!/bin/bash
# Search for gen_json_dict_str in parent case.Case
rg -n "def gen_json_dict_str" toolchain/mfc/case.py -B3 -A15Length of output: 631 🏁 Script executed: #!/bin/bash
# Check for any test that might trigger probe placement without domain keys
# Look for cases with n=0 or p=0 and empty or minimal mods
rg -n "define_case_d.*n.*0|define_case_f" toolchain/mfc/test/cases.py -B2 -A5 | head -80Length of output: 4077 🏁 Script executed: #!/bin/bash
# Search for any get_parameters or similar that might show what goes into case dict
rg -n "get_parameters|params" toolchain/mfc/test/case.py -B2 -A5 | head -40Length of output: 1842 Length of output: 5523 🏁 Script executed: #!/bin/bash
# Search for x_domain in BASE_CFG section
rg -n "x_domain" toolchain/mfc/test/case.py -B3 -A3Length of output: 1447 🏁 Script executed: #!/bin/bash
# Check TestCaseBuilder.to_case() to see how it loads params
rg -n "def to_case" toolchain/mfc/test/case.py -A30Length of output: 1038 🏁 Script executed: #!/bin/bash
# Look for test cases that might have n=0 or p=0 (1D/2D) without explicit domain mods
rg -n "stack.push.*1D|n.*:.*0|p.*:.*0" toolchain/mfc/test/cases.py -B2 -A8 | head -100Length of output: 6890 Add guards for missing domain keys or document requirement The code at lines 241, 246–256, 260–262 accesses domain keys ( Recommendations:
|
||
| print(json.dumps({{**case, **mods}})) | ||
| """) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.