Add CI for tracking porting progress using gcov and custom parsing + report generation - #80
Merged
Merged
Conversation
Author
|
noting this pr, once merged, will just get the site generation working. I'll put in another follow-up PR that adds:
|
Member
|
Looks really nice, good job Claude! I'm guessing there are false positives, and maybe there's some overhead on the CI, but we can play with it for a while to work out those issues. The plan is to remove the code changes before merging right? |
track_gpu_port.py cross-references gcov execution coverage against GPU-ported source regions (do concurrent, omp target directives, manual @gpu-ported/@start noport/toport markers) to report which executed, GPU-portable lines still need porting. gpu-port-report.yml runs it in CI against a short benchmark_ALE run built with --coverage instrumentation.
…parate build .testing's build/cov/MOM6 is already compiled with --coverage from the same PR source, the same FMS fork (edoyango/FMS) that MOM6-examples/src/FMS2_gpu points to, and the default dynamic_symmetric/solo_driver config that ocean_only uses — so it can run benchmark_ALE directly with no recompile. Moves the report job into verify-linux.yml (needs: build-coverage) since `needs` can only reference jobs in the same workflow file; the standalone gpu-port-report.yml is removed. Cloning MOM6-gpu/MOM6-examples is now only for the benchmark_ALE input files and the f90nml submodule (for get_nprocs.py) — no FMS/MOM6 build there at all.
CI failed: get_nprocs.py computed 64 ranks from MOM6-gpu/MOM6-examples's checked-in MOM_parameter_doc.layout for benchmark_ALE, exceeding the runner's available slots. This is a coverage smoke run, not a scaling test, so hardcode -np 1 and force "#override LAYOUT = 1,1" so the runtime domain decomposition matches. Drops the now-unnecessary get_nprocs.py call and its f90nml submodule dependency.
CI failed with FileNotFoundError on src/equation_of_state/TEOS10/*.f90: those are git symlinks into the pkg/GSW-Fortran submodule, and gpu-port-report's checkout doesn't initialize submodules (the coverage build itself never compiles them — it links an externally-built libgsw.a instead via --with-gsw). rglob() still walks the dangling symlinks, so skip any path that isn't a real file, with a warning, instead of crashing the whole report.
track_gpu_port.py gains --baseline-json (diff current ported totals against a prior --out-json snapshot) and --changed-files + --out-comment (a short summary scoped to the PR's own files, for posting as a comment rather than the full per-file/per-routine report). verify-linux.yml's gpu-port-report job now uploads its JSON snapshot as a "gpu-port-json" artifact on every run (the next PR's baseline), and on pull_request events also computes the PR's changed .F90/.f90 files and looks up the base branch's latest snapshot to diff against. Actually posting the comment is split into a new gpu-port-comment.yml, triggered by workflow_run: gpu-port-report checks out and runs PR code (possibly from a fork) so it must stay read-only, while commenting needs write access that workflow_run grants safely since it runs with the base repo's permissions and never checks out the triggering code. The comment is updated in place across pushes via a hidden HTML marker.
…port Without it, the "top porting opportunities" section in --out-md caps at 25 routines by portable-remaining lines, which is fine for a quick scan but drops routines once the codebase has more than 25 with remaining work. --verbose lists all of them, with their exact line ranges. Wired into the CI job's full report (job summary), not the PR comment, which stays short by design.
Adds --out-html to track_gpu_port.py, generating a self-contained line-by-line coverage/port-status page per source file (ported / portable-remaining / executed-not-portable / not-hit, plus an index). The read-only gpu-port-report job builds it as an artifact; the write-capable gpu-port-comment.yml workflow (via workflow_run) pushes it to gh-pages under pr-<number>/ and links it from the sticky PR comment. A pull_request_target cleanup workflow removes the preview when the PR closes.
Green stays "done", but "portable, not yet ported" (the actual action item) is now red, "executed, not portable" (benign — no porting target) is blue instead of yellow to avoid reading as a warning, and "executable, not hit" is grey.
Previously the GPU port HTML report was only ever published per-PR (pr-<number>/) and got deleted again once the PR closed, so merging lost the report instead of updating a durable one. gpu-port-report now also runs on pushes to dev/gpu, and gpu-port-comment.yml publishes those to a fixed porting_progress/ path on gh-pages instead of a PR-numbered one.
The "publish-pages" job downloaded the report artifact into "report/" (inside the checked-out repo), then later ran "git add -A" when committing to gh-pages. That staged the whole working tree, not just the intended publish path and index.html, so the first successful run accidentally committed "report/" itself into gh-pages. Every subsequent run then failed: the fresh, untracked "report/" download collided with the tracked copy already on gh-pages, and "git checkout -B gh-pages FETCH_HEAD" aborted rather than overwrite it. Downloading to $RUNNER_TEMP instead means the artifact can never again collide with whatever's checked out, and scoping "git add" to exactly the publish path plus index.html means a stray leftover elsewhere in the workdir can't get swept into a gh-pages commit either. Does not by itself unblock the branch: gh-pages already has "report/" committed to it from the earlier bad run, so a one-time cleanup commit removing it from gh-pages is still needed before publishing will succeed again.
Author
yes sorry I've removed the code changes just now |
marshallward
approved these changes
Jul 28, 2026
marshallward
left a comment
marshallward
left a comment
Member
There was a problem hiding this comment.
The @noport @toport stuff may need to be dropped as we look to syncing with dev/gfdl, but we can start thinking about that later on.
Member
|
The commits are well-structured so I'll do a rebase. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This creates a new CI that generates a report for both the repo's default branch and any pr that comes in
gh-pages(will be created if it doesn't exist)gh-pageshistory may need pruning at some point.The report uses gcov, which means code only hit by benchmark_ALE is shown in the report.
An example of what the report will look like is here: https://edoyango.github.io/MOM6/
Latest on dev/gpuis the report for dev/gpu, andpr-2is a PR example (the PR is small, it just updates the script and adds some annotations to MOM_barotropic.F90).NB: Only once this PR is merged, will the CI start to trigger.
This PR was coded entirely with claude sonnet 5