Improve adjoint test hygiene #3278

Activity

1. smartalecH

   added

   adjoint optimization

   unit tests

   on Aug 25, 2026
2. jin-castle commented on Aug 25, 2026

   

   jin-castle

   on Aug 25, 2026

   Contributor

   More actions

   This is very much aligned with what I've been running into while working on
   #3263/#3274 — happy to help here.

   One suggestion for the consolidation step: if the unified driver emits a small
   structured record per case (objective, simulation dim, design-grid dim,
   boundary, precision, measured error, tolerance), the test matrix in point 1
   stops being documentation and becomes a CI output. From that same record, CI
   can render:

   • the coverage matrix (tested / passing / missing combinations);
   • measured error vs. allowed tolerance per case, which makes tolerance drift
     visible — passes that consume >50% of their tolerance are exactly the ones
     worth auditing before unifying thresholds;
   • for failed or near-tolerance cases, a spatial residual map over the design
     grid (the chunk-partition bug in [1/2] Fix adjoint gradient chunk-dependence without gathering the DFT monitors #3274 shows up as a block pattern at chunk
     boundaries, which is far more diagnostic than a scalar L2 error).

   Mockup of what this could look like (illustrative data):

   

   Recording the measured error per case also forces the refactor that point 3
   needs anyway — most current tests assert deep inside and never expose the
   actual error.

   Assertions would remain the sole source of pass/fail; reporting would be
   opt-in via an environment variable so local make check-adjoint is unchanged,
   and matplotlib is already in python/requirements.txt so there are no new
   dependencies. A first PR could be just the record schema + a GitHub Actions
   job summary, with the plots as a follow-up.

   Do you already have a shape in mind for the unified driver? I'd rather fit
   the record schema to that than design it separately.
3. smartalecH commented on Aug 27, 2026

   

   smartalecH

   on Aug 27, 2026

   CollaboratorAuthor

   More actions

   Thanks! I think a visual dashboard like what you describe above would be fantastic. Some challenges we might hit (would like to hear your thoughts wrt how we could tackle them):

   • Is there a clean way to integrate this within our CI? How can we access a dashboard like this with the current setup?
   • Adjoint tests are really expensive (especially the 3D) and we don't run them all by default in CI. Is there a way we can optionally trigger a full "dashboard" run on a PR or branch (e.g. right before we merge)?
4. jin-castle commented on Sep 5, 2026

   

   jin-castle

   on Sep 5, 2026

   Contributor

   More actions

   Both are answerable against build-ci-adjoint.yml as it stands, which is
   convenient — I don't think either needs new infrastructure.

   1. Where the dashboard lives

   The GitHub Actions job summary ($GITHUB_STEP_SUMMARY) is the piece that
   makes this work without hosting anything. Anything a step appends to that file
   is rendered as markdown on the run's own page, directly under the job. So the
   coverage matrix and the tolerance-utilization table become part of the run:
   no external service, no pages branch, no token, no permissions beyond the
   default read-only GITHUB_TOKEN — which also means it works unchanged on pull
   requests from forks, where a bot-comment approach would need
   pull_request_target and the security review that comes with it.

   The mechanics, on top of the existing workflow:

   1. Set MEEP_ADJOINT_REPORT=${{ runner.temp }}/adjoint-records on the existing
      Run adjoint tests step. Unset — which is every local make check-adjoint
      — the tests behave exactly as they do now.
   2. Each case appends one JSON line: objective, sim dim, design-grid dim,
      boundary, precision, measured error, tolerance.
   3. A new step with if: always() renders those records to
      $GITHUB_STEP_SUMMARY. always() matters: the interesting run is the one
      where something failed.

   Two limits worth designing around rather than hitting: the summary is capped at
   1 MiB per step, and it renders markdown, not images. That splits the output
   naturally — tables go to the summary, plots go to artifacts. The residual
   heatmaps become an actions/upload-artifact@v7 upload, which the workflow
   already does for test-suite.log; it is the same step with if: failure()
   relaxed to if: always() for the report bundle. Anyone reading the run
   downloads one zip.

   If a summary table ever needs to be visible from the PR conversation rather than
   the run page, that is a later, separable step and I'd rather not pay for it up
   front.

   2. Triggering the expensive run

   The workflow already carries workflow_dispatch and a weekly schedule, so the
   two expensive-run stories are mostly a matter of gating:

   • On demand, any branch. Give workflow_dispatch an input (e.g.
     suite: quick | full) so a maintainer can start a full run from the Actions
     tab against a branch without pushing anything.

   • Right before merge, from the PR. Add labeled to the pull_request
     trigger types and gate the expensive cases on the label:

     on:
       pull_request:
         types: [opened, synchronize, reopened, labeled]

     if: >-
       github.event_name == 'workflow_dispatch' ||
       github.event_name == 'schedule' ||
       contains(github.event.pull_request.labels.*.name, 'full-adjoint')

     Applying the full-adjoint label then is the "run the dashboard before we
     merge" gesture, and removing and re-adding it re-runs. Note this stays on
     pull_request, not pull_request_target, so fork PRs keep a read-only token.

   • Always-on baseline. The existing Sunday schedule entry runs the full
     suite on master, which is what makes tolerance drift visible over time — a
     single run's tolerance-utilization column is much less informative than the
     same column a month apart.

   • Worth adding while touching the triggers:

     concurrency:
       group: adjoint-${{ github.workflow }}-${{ github.ref }}
       cancel-in-progress: true

     so pushing three times in ten minutes to a full-adjoint-labelled PR does not
     queue three full runs.

   One thing that makes the cost question smaller than it looks: emitting the
   record costs nothing — it is a few JSON lines from tests that are already
   running, so the default PR run gets the coverage matrix and the tolerance
   column for free. What the label gates is the additional expensive cases (3d,
   MPI, single precision), not the reporting. That also means the schema gets
   exercised on every PR from day one instead of only on labelled runs, which is
   the failure mode I'd worry about most for a reporting path.

   Suggested order

   1. Record schema + the if: always() summary renderer, wired into the existing
      serial job. Small, and it makes the coverage matrix real immediately.
   2. Trigger/label gating and the expensive matrix.
   3. Residual heatmaps as artifacts.

   Happy to take (1) as a PR against whatever the unified driver turns out to be —
   which is the part I'd still rather fit to your design than guess at. If the
   driver is going to be a parameterized matrix over a single case class, the
   record is one dataclass and one asdict at teardown, and I can write it
   against your skeleton as soon as there is one.