Skip to content

Commit 85c715e

Browse files
sbryngelsonclaude
andcommitted
Fix dedup large-domain math, add --host flag, add multi-rank assembly tests
#3: Remove max(0,...) clamp from _dedup decimals formula so that large-extent domains (>1e12) use negative decimal rounding (numpy np.round supports this) instead of collapsing all coordinates to 0. #6: Add TestMultiRankAssembly with two synthetic tests: - two_rank_1d_dedup: verifies ghost-cell overlap is removed and the assembled variable array has the correct 4-cell result - large_extent_dedup: exercises the negative-decimals code path (scale=1e7) to confirm deduplication works at large domain extents #8: Add --host argument (default 127.0.0.1) to the interactive Dash server so users can bind to 0.0.0.0 for direct HPC access without SSH tunneling. Only show the SSH tunnel hint when host is localhost. Addresses Claude Code review findings on PR #1233. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 189dc24 commit 85c715e

5 files changed

Lines changed: 78 additions & 5 deletions

File tree

toolchain/mfc/cli/commands.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,6 +1031,12 @@
10311031
default=8050,
10321032
metavar="PORT",
10331033
),
1034+
Argument(
1035+
name="host",
1036+
help="Host address for the interactive web server (default: 127.0.0.1).",
1037+
default="127.0.0.1",
1038+
metavar="HOST",
1039+
),
10341040
Argument(
10351041
name="tui",
10361042
help="Launch an interactive terminal UI (1D/2D only). Works over SSH with no browser.",

toolchain/mfc/viz/interactive.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ def run_interactive( # pylint: disable=too-many-locals,too-many-statements
206206
steps: List[int],
207207
read_func: Callable,
208208
port: int = 8050,
209+
host: str = '127.0.0.1',
209210
):
210211
"""Launch the interactive Dash visualization server."""
211212
app = Dash(
@@ -613,7 +614,8 @@ def _tf(arr): return arr
613614

614615
# ------------------------------------------------------------------
615616
cons.print(f'\n[bold green]Interactive viz server:[/bold green] '
616-
f'[bold]http://localhost:{port}[/bold]')
617-
cons.print(f'[dim]SSH tunnel: ssh -L {port}:localhost:{port} <hostname>[/dim]')
617+
f'[bold]http://{host}:{port}[/bold]')
618+
if host in ('127.0.0.1', 'localhost'):
619+
cons.print(f'[dim]SSH tunnel: ssh -L {port}:localhost:{port} <hostname>[/dim]')
618620
cons.print('[dim]Ctrl+C to stop.[/dim]\n')
619-
app.run(debug=False, port=port, host='127.0.0.1')
621+
app.run(debug=False, port=port, host=host)

toolchain/mfc/viz/reader.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,10 +318,12 @@ def assemble_from_proc_data( # pylint: disable=too-many-locals,too-many-stateme
318318
# Build unique sorted global coordinate arrays (handles ghost overlap).
319319
# Use scale-aware rounding: 12 significant digits relative to the domain
320320
# extent, so precision is preserved for both micro-scale and large domains.
321+
# np.round supports negative decimals (rounds to tens, hundreds, etc.),
322+
# which is correct for large-extent domains (e.g. extent > 1e12).
321323
def _dedup(arr):
322324
extent = arr.max() - arr.min()
323325
if extent > 0:
324-
decimals = max(0, int(np.ceil(-np.log10(extent))) + 12)
326+
decimals = int(np.ceil(-np.log10(extent))) + 12
325327
else:
326328
decimals = 12
327329
return np.unique(np.round(arr, decimals)), decimals

toolchain/mfc/viz/test_viz.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,5 +531,66 @@ def test_render_2d_log_scale(self):
531531
os.unlink(out)
532532

533533

534+
# ---------------------------------------------------------------------------
535+
# Tests: multi-rank assembly (ghost-cell deduplication)
536+
# ---------------------------------------------------------------------------
537+
538+
class TestMultiRankAssembly(unittest.TestCase):
539+
"""Test assemble_from_proc_data with synthetic multi-processor data."""
540+
541+
def _make_proc(self, x_cb, pres):
542+
"""Build a minimal 1D ProcessorData from boundary coordinates."""
543+
import numpy as np
544+
from .reader import ProcessorData
545+
return ProcessorData(
546+
m=len(x_cb) - 1,
547+
n=0,
548+
p=0,
549+
x_cb=np.array(x_cb, dtype=np.float64),
550+
y_cb=np.array([0.0]),
551+
z_cb=np.array([0.0]),
552+
variables={'pres': np.array(pres, dtype=np.float64)},
553+
)
554+
555+
def test_two_rank_1d_dedup(self):
556+
"""Two processors with one overlapping ghost cell assemble correctly."""
557+
import numpy as np
558+
from .reader import assemble_from_proc_data
559+
# Domain: 4 cells with centers at 0.125, 0.375, 0.625, 0.875
560+
# Proc 0 sees cells 0-2 (center 0.625 is ghost from proc 1)
561+
# Proc 1 sees cells 1-3 (center 0.375 is ghost from proc 0)
562+
p0 = self._make_proc([0.00, 0.25, 0.50, 0.75],
563+
[1.0, 2.0, 3.0]) # centers: 0.125, 0.375, 0.625
564+
p1 = self._make_proc([0.25, 0.50, 0.75, 1.00],
565+
[2.0, 3.0, 4.0]) # centers: 0.375, 0.625, 0.875
566+
567+
result = assemble_from_proc_data([(0, p0), (1, p1)])
568+
569+
self.assertEqual(result.ndim, 1)
570+
self.assertEqual(len(result.x_cc), 4)
571+
np.testing.assert_allclose(result.x_cc, [0.125, 0.375, 0.625, 0.875])
572+
np.testing.assert_allclose(result.variables['pres'], [1.0, 2.0, 3.0, 4.0])
573+
574+
def test_large_extent_dedup(self):
575+
"""Deduplication works correctly for large-extent domains (>1e6)."""
576+
import numpy as np
577+
from .reader import assemble_from_proc_data
578+
# Scale up by 1e7 to exercise the negative-decimals code path
579+
scale = 1e7
580+
p0 = self._make_proc(
581+
[0.00 * scale, 0.25 * scale, 0.50 * scale, 0.75 * scale],
582+
[1.0, 2.0, 3.0],
583+
)
584+
p1 = self._make_proc(
585+
[0.25 * scale, 0.50 * scale, 0.75 * scale, 1.00 * scale],
586+
[2.0, 3.0, 4.0],
587+
)
588+
result = assemble_from_proc_data([(0, p0), (1, p1)])
589+
self.assertEqual(len(result.x_cc), 4)
590+
np.testing.assert_allclose(
591+
result.variables['pres'], [1.0, 2.0, 3.0, 4.0]
592+
)
593+
594+
534595
if __name__ == "__main__":
535596
unittest.main()

toolchain/mfc/viz/viz.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,9 +259,11 @@ def read_step(step):
259259
if interactive:
260260
from .interactive import run_interactive # pylint: disable=import-outside-toplevel
261261
port = ARG('port')
262+
host = ARG('host')
262263
# Default to first available variable if --var was not specified
263264
init_var = varname if varname in avail else (avail[0] if avail else None)
264-
run_interactive(init_var, requested_steps, read_step, port=int(port))
265+
run_interactive(init_var, requested_steps, read_step,
266+
port=int(port), host=str(host))
265267
return
266268

267269
# Validate colormap before any rendering

0 commit comments

Comments
 (0)