From 018214d20ad423912cfb8f5d2d2b9cdd58aa978f Mon Sep 17 00:00:00 2001 From: Brianna Major Date: Mon, 17 Aug 2026 09:56:12 -0400 Subject: [PATCH 1/5] Fail PHOLD runs that produce no output files instead of marking them completed --- net_maestro/core/management/commands/run_phold.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/net_maestro/core/management/commands/run_phold.py b/net_maestro/core/management/commands/run_phold.py index 63de605..5872767 100644 --- a/net_maestro/core/management/commands/run_phold.py +++ b/net_maestro/core/management/commands/run_phold.py @@ -329,6 +329,19 @@ def _ingest_output_files( # Process any additional files _process_additional_files(context, processed_files, task_signatures, immediate=immediate) + if not processed_files: + # None of the expected output files were produced. A genuinely successful run always writes + # at least the gvt/rt stats files. + click.echo( + "Error: No expected PHOLD output files were found in " + f"{actual_output_dir}. Marking run as failed.", + err=True, + ) + mark_run_failed( + None, RuntimeError("No PHOLD output files were produced"), None, run_id=run.id + ) + return + # Mark the run as completed once ingestion is done. if immediate: # Ingestion already ran synchronously inside _ingest_output_file; mark now. From e4e77fa7b1cbae3003b6a408d9738bd1e51cf7ea Mon Sep 17 00:00:00 2001 From: Brianna Major Date: Mon, 17 Aug 2026 09:59:21 -0400 Subject: [PATCH 2/5] Build PHOLD into the Docker image instead of mounting a host ROSS checkout --- dev/.env.docker-compose | 9 +++------ dev/django.Dockerfile | 35 +++++++++++++++++++++++++++++++++-- dev/docker-development.md | 2 +- docker-compose.override.yml | 2 -- 4 files changed, 37 insertions(+), 11 deletions(-) diff --git a/dev/.env.docker-compose b/dev/.env.docker-compose index 411fde7..b69a373 100644 --- a/dev/.env.docker-compose +++ b/dev/.env.docker-compose @@ -10,14 +10,11 @@ DJANGO_MINIO_STORAGE_MEDIA_URL=http://localhost:9000/django-storage # access from non-internal addresses. DJANGO_INTERNAL_IPS=0.0.0.0/0 -# Path to ROSS simulation framework directory -# ROSS_PATH=../ross - # PHOLD simulation configuration -# DJANGO_PHOLD_BINARY_PATH=/home/vscode/ross/build/models/phold/phold +DJANGO_PHOLD_BINARY_PATH=/opt/ross/phold # Directory for PHOLD output files -# DJANGO_PHOLD_OUTPUT_DIR=/tmp/phold_output +DJANGO_PHOLD_OUTPUT_DIR=/tmp/phold_output # Number of MPI processes for PHOLD simulations (set to 1 for no MPI) -# DJANGO_PHOLD_MPI_PROCESSES=4 +DJANGO_PHOLD_MPI_PROCESSES=4 diff --git a/dev/django.Dockerfile b/dev/django.Dockerfile index 7e0ef7e..de329fa 100644 --- a/dev/django.Dockerfile +++ b/dev/django.Dockerfile @@ -1,13 +1,44 @@ +# Pin to a specific ROSS commit for reproducible builds. +ARG ROSS_GIT_REF=dc3a6a056cfc7a5e68f7141f88d8833407599ef8 + +FROM ubuntu:24.04 AS ross-builder +ARG ROSS_GIT_REF + +RUN apt-get update && apt-get install -y --no-install-recommends \ + ca-certificates \ + git \ + build-essential \ + cmake \ + openmpi-bin \ + libmpich-dev \ + libopenmpi-dev \ + && rm -rf /var/lib/apt/lists/* + +RUN git clone https://github.com/ross-org/ross.git /ross \ + && cd /ross \ + && git checkout "${ROSS_GIT_REF}" \ + && git submodule update --init --recursive + +RUN cmake -S /ross -B /ross/build \ + -DCMAKE_BUILD_TYPE=Release \ + -DROSS_BUILD_MODELS=ON \ + -DROSS_BUILD_TESTING=OFF \ + && cmake --build /ross/build --parallel + + FROM mcr.microsoft.com/devcontainers/base:ubuntu-24.04 -# Install OpenMPI for simulations +# OpenMPI runtime, matching the ross-builder stage, for running the PHOLD binary. RUN sudo apt-get update && sudo apt-get install -y \ openmpi-bin \ - libopenmpi-dev \ && sudo apt-get clean && sudo rm -rf /var/lib/apt/lists/* COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /usr/local/bin/ +# PHOLD binary built in the ross-builder stage above. Baked into the image instead +# of relying on a host `ross` checkout bind-mounted at runtime. +COPY --from=ross-builder --chown=vscode:vscode /ross/build/models/phold/phold /opt/ross/phold + # Ensure Python output appears immediately in container logs. ENV PYTHONUNBUFFERED=1 diff --git a/dev/docker-development.md b/dev/docker-development.md index a2ec8a3..d3898ae 100644 --- a/dev/docker-development.md +++ b/dev/docker-development.md @@ -3,7 +3,7 @@ An alternative to the recommended [dev container](../README.md) workflow. ## Setup -1. **Configure simulation paths**: Set `ROSS_PATH` and PHOLD settings in `dev/.env.docker-compose` +1. **Configure simulation settings** (optional): PHOLD settings in `dev/.env.docker-compose`. 1. `docker compose run --rm django ./manage.py migrate` 1. `docker compose run --rm django ./manage.py createsuperuser` diff --git a/docker-compose.override.yml b/docker-compose.override.yml index 613169f..a62b853 100644 --- a/docker-compose.override.yml +++ b/docker-compose.override.yml @@ -13,7 +13,6 @@ services: env_file: ./dev/.env.docker-compose volumes: - .:/home/vscode/net-maestro - - ${ROSS_PATH:-../ross}:/home/vscode/ross - pkg_cache:/home/vscode/pkg-cache ports: - 8000:8000 @@ -45,7 +44,6 @@ services: env_file: ./dev/.env.docker-compose volumes: - .:/home/vscode/net-maestro - - ${ROSS_PATH:-../ross}:/home/vscode/ross - pkg_cache:/home/vscode/pkg-cache depends_on: postgres: From 7b86831c4f40b8c5071c41ba68fb3617f50c7bc6 Mon Sep 17 00:00:00 2001 From: Brianna Major Date: Fri, 28 Aug 2026 14:04:42 -0400 Subject: [PATCH 3/5] Build MPICH from source for the ROSS image instead of apt OpenMPI --- dev/django.Dockerfile | 46 ++++++++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/dev/django.Dockerfile b/dev/django.Dockerfile index de329fa..23851be 100644 --- a/dev/django.Dockerfile +++ b/dev/django.Dockerfile @@ -5,15 +5,31 @@ FROM ubuntu:24.04 AS ross-builder ARG ROSS_GIT_REF RUN apt-get update && apt-get install -y --no-install-recommends \ - ca-certificates \ - git \ - build-essential \ - cmake \ - openmpi-bin \ - libmpich-dev \ - libopenmpi-dev \ + ca-certificates git \ + build-essential clang \ + cmake ninja-build \ + pkg-config flex \ + bison lcov wget \ && rm -rf /var/lib/apt/lists/* +# --- MPICH from source, ch4:ofi device (embedded libfabric, sockets/TCP). No UCX +# and no verbs probing, reliable PMI wire-up -> mpiexec forms a real multi- +# rank world on the CI runners. Installed to /opt/mpich and put on PATH so +# find_package(MPI) discovers mpicc/mpicxx/mpiexec with no CODES/ROSS config +# change. Bump MPICH_VERSION to update -- that's the whole maintenance story. +ARG MPICH_VERSION=4.3.2 +RUN wget -q "https://www.mpich.org/static/downloads/${MPICH_VERSION}/mpich-${MPICH_VERSION}.tar.gz" \ + && tar xzf "mpich-${MPICH_VERSION}.tar.gz" \ + && cd "mpich-${MPICH_VERSION}" \ + && ./configure --prefix=/opt/mpich --with-device=ch4:ofi --disable-fortran \ + && make -j"$(nproc)" \ + && make install \ + && cd / \ + && rm -rf "mpich-${MPICH_VERSION}" "mpich-${MPICH_VERSION}.tar.gz" + +# Put our mpich first so find_package(MPI) picks it over anything else. +ENV PATH=/opt/mpich/bin:$PATH + RUN git clone https://github.com/ross-org/ross.git /ross \ && cd /ross \ && git checkout "${ROSS_GIT_REF}" \ @@ -25,16 +41,22 @@ RUN cmake -S /ross -B /ross/build \ -DROSS_BUILD_TESTING=OFF \ && cmake --build /ross/build --parallel - FROM mcr.microsoft.com/devcontainers/base:ubuntu-24.04 -# OpenMPI runtime, matching the ross-builder stage, for running the PHOLD binary. -RUN sudo apt-get update && sudo apt-get install -y \ - openmpi-bin \ - && sudo apt-get clean && sudo rm -rf /var/lib/apt/lists/* +# Declared (empty by default) so the LD_LIBRARY_PATH append below references a +# variable defined in this stage, rather than an inherited/unset one. +ARG LD_LIBRARY_PATH= COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /usr/local/bin/ +# MPICH runtime (mpiexec/mpirun + libmpi), built from source in the ross-builder +# stage above. Must be the *same* MPICH build phold was linked against -- an +# apt-installed OpenMPI or a different MPICH build here would reintroduce the +# ABI/PMI-handshake mismatch this custom build exists to avoid. +COPY --from=ross-builder --chown=vscode:vscode /opt/mpich /opt/mpich +ENV PATH=/opt/mpich/bin:$PATH \ + LD_LIBRARY_PATH=/opt/mpich/lib${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} + # PHOLD binary built in the ross-builder stage above. Baked into the image instead # of relying on a host `ross` checkout bind-mounted at runtime. COPY --from=ross-builder --chown=vscode:vscode /ross/build/models/phold/phold /opt/ross/phold From 9c47997c56951bd42cfe307e6251803f2ba34636 Mon Sep 17 00:00:00 2001 From: Brianna Major Date: Fri, 28 Aug 2026 14:04:48 -0400 Subject: [PATCH 4/5] Extract fail_run so runs can fail outside the Celery callback path --- .../core/management/commands/run_phold.py | 6 ++-- net_maestro/core/tasks/simulation.py | 29 +++++++++++-------- .../core/tests/test_simulation_tasks.py | 17 +++++++---- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/net_maestro/core/management/commands/run_phold.py b/net_maestro/core/management/commands/run_phold.py index 5872767..73cd595 100644 --- a/net_maestro/core/management/commands/run_phold.py +++ b/net_maestro/core/management/commands/run_phold.py @@ -22,7 +22,7 @@ from net_maestro.core.constants import RunStatus from net_maestro.core.models import EventFile, ModelFile, Run, SimulationFile from net_maestro.core.tasks import run_event_task, run_model_task, run_simulation_task -from net_maestro.core.tasks.simulation import mark_run_completed, mark_run_failed +from net_maestro.core.tasks.simulation import fail_run, mark_run_completed, mark_run_failed @dataclass @@ -337,9 +337,7 @@ def _ingest_output_files( f"{actual_output_dir}. Marking run as failed.", err=True, ) - mark_run_failed( - None, RuntimeError("No PHOLD output files were produced"), None, run_id=run.id - ) + fail_run(run.id, "No PHOLD output files were produced") return # Mark the run as completed once ingestion is done. diff --git a/net_maestro/core/tasks/simulation.py b/net_maestro/core/tasks/simulation.py index 778e781..ae7d64f 100644 --- a/net_maestro/core/tasks/simulation.py +++ b/net_maestro/core/tasks/simulation.py @@ -38,6 +38,22 @@ def mark_run_completed(run_id: int) -> None: logger.exception("Run %s not found when trying to mark as completed", run_id) +def fail_run(run_id: int, reason: str) -> None: + """Mark a run as FAILED, logging the reason. + + Shared by ``mark_run_failed`` (the Celery error-callback path) and any + caller that needs to fail a run for a reason that isn't a Celery task + exception (e.g. a validation failure discovered synchronously). + """ + logger.error("Run %s failed: %s", run_id, reason) + try: + run = Run.objects.get(pk=run_id) + run.status = RunStatus.FAILED + run.save() + except Run.DoesNotExist: + logger.exception("Run %s not found when trying to mark as failed", run_id) + + @shared_task def mark_run_failed(request: object, exc: Exception, traceback: object, *, run_id: int) -> None: """Mark a run as failed when an ingestion task in the chain errors. @@ -48,18 +64,7 @@ def mark_run_failed(request: object, exc: Exception, traceback: object, *, run_i task's ``request``, the raised ``exc``, and its ``traceback``; ``run_id`` is bound as a keyword argument when the callback is attached. """ - logger.error( - "Run %s ingestion task %s failed: %r", - run_id, - getattr(request, "id", "unknown"), - exc, - ) - try: - run = Run.objects.get(pk=run_id) - run.status = RunStatus.FAILED - run.save() - except Run.DoesNotExist: - logger.exception("Run %s not found when trying to mark as failed", run_id) + fail_run(run_id, f"ingestion task {getattr(request, 'id', 'unknown')} failed: {exc!r}") @shared_task diff --git a/net_maestro/core/tests/test_simulation_tasks.py b/net_maestro/core/tests/test_simulation_tasks.py index a9f8b33..4dce1e6 100644 --- a/net_maestro/core/tests/test_simulation_tasks.py +++ b/net_maestro/core/tests/test_simulation_tasks.py @@ -225,15 +225,19 @@ def test_chain_creation_with_files( # Verify chain was executed mock_workflow.apply_async.assert_called_once() - @mock.patch("net_maestro.core.management.commands.run_phold.mark_run_completed") @mock.patch("net_maestro.core.management.commands.run_phold._execute_phold") - def test_no_files_still_marks_completed( + def test_no_files_marks_failed( self, mock_exec_phold: mock.Mock, - mock_mark_completed: mock.Mock, tmp_path: Path, + caplog: pytest.LogCaptureFixture, ) -> None: - """Test that run is marked completed even with no files to ingest.""" + """Test that a run with no output files is marked failed, not completed. + + A genuinely successful PHOLD run always writes at least the gvt/rt stats + files, so an empty output directory indicates the simulation didn't + actually produce results and the run must not be marked COMPLETED. + """ # Setup empty output directory output_dir = tmp_path / "output" output_dir.mkdir() @@ -261,5 +265,6 @@ def test_no_files_still_marks_completed( str(output_dir), ) - # Verify mark_run_completed was called directly - mock_mark_completed.delay.assert_called_once_with(run.id) + run.refresh_from_db() + assert run.status == RunStatus.FAILED + assert f"Run {run.id} failed: No PHOLD output files were produced" in caplog.text From d30c7301b2d16a19aea2dfd68c8986114019b80f Mon Sep 17 00:00:00 2001 From: Brianna Major Date: Fri, 28 Aug 2026 14:04:52 -0400 Subject: [PATCH 5/5] Clarify the optional PHOLD settings step in docker-development.md --- dev/docker-development.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/docker-development.md b/dev/docker-development.md index d3898ae..b55a952 100644 --- a/dev/docker-development.md +++ b/dev/docker-development.md @@ -3,7 +3,7 @@ An alternative to the recommended [dev container](../README.md) workflow. ## Setup -1. **Configure simulation settings** (optional): PHOLD settings in `dev/.env.docker-compose`. +1. **Configure simulation settings** (optional): Review and change PHOLD settings in `dev/.env.docker-compose`. 1. `docker compose run --rm django ./manage.py migrate` 1. `docker compose run --rm django ./manage.py createsuperuser`