Skip to content

WORKFLOWS.md: CTParser.jl misclassified as having no GPU code, and no guidance on split-CI label naming #10

Description

@jbcaillau

Follow-up to #9 / #8. Bringing CTParser.jl up to date with the handbook (control-toolbox/CTParser.jl#302) surfaced two problems in WORKFLOWS.md. #9 noted in passing that "CTDirect.jl/CTParser.jl are not currently kept up to date with the rest of the fleet" — that is no longer true of CTParser, which changes the facts underlying both items below.


1. §3.1 and §4 wrongly state that CTParser.jl has no GPU-relevant code

§3.1 currently ends with:

A package with no GPU-relevant code (CTModels.jl, CTParser.jl, CTLie for now) keeps the single call job gated by plain run ci.

and the §4 table row is:

Repo Base set Breakage AddToProject Extra
CTParser.jl

i.e. no split CI (cpu/gpu) in the Extra column, unlike CTFlows.jl / CTSolvers / OptimalControl.

This is factually wrong. CTParser has substantial GPU/ExaModels-facing code:

  • test/Project.toml [deps] includes CUDA, MadNLPGPU, KernelAbstractions and ExaModels (with [compat] entries CUDA = "5", MadNLPGPU = "0.8", KernelAbstractions = "0.9").
  • test/runtests.jl loads them unconditionally: using ExaModels, using MadNLP, using MadNLPGPU, using CUDA.
  • Four dedicated test files: test_dynamics_exa.jl, test_exa_linalg.jl, test_onepass_exa.jl, test_onepass_exa_bis.jl.
  • src/ carries the ExaModels backend itself — src/onepass.jl has the backend-prefix machinery, discretise_exa, and explicit GPU handling (Array(...) # conversion to Array for GPU, Array($pref.multipliers(...)) # Array to copy from GPU); src/defaults.jl exposes the default ExaModels backend/scheme/grid size.

So CTParser belongs in the split-CI group, not in the "no GPU-relevant code" group. As of control-toolbox/CTParser.jl#302 it now runs a two-job CI.yml modelled on CTFlows.jl.

Suggested fix

  • Drop CTParser.jl from the "no GPU-relevant code" sentence in §3.1 (leaving CTModels.jl and CTLie), and add it to the list of packages already using the split alongside CTFlows.jl and CTSolvers.
  • Update the §4 row to split CI (github/kkt) in the Extra column.

2. §2 gives no guidance for split-CI label naming, so a conforming repo can silently diverge

The §2 label table sanctions exactly one vocabulary for a split CI:

Label Triggers Used in
run ci cpu CI on GitHub-hosted CPU runners packages with split CI: OptimalControl, CTFlows.jl, CTSolvers (CTLie planned)
run ci gpu CI on self-hosted GPU runner packages with split CI: OptimalControl, CTFlows.jl, CTSolvers (CTLie planned)

CTParser.jl has deliberately adopted github-runner and kkt-runner instead — naming the split by runner rather than by workload, which is arguably the more accurate description (the GitHub-hosted job is not meaningfully "the CPU job"; it is the job that runs on GitHub's runners, and it happens to exercise CPU paths).

The handbook is currently silent on whether this is permitted, so there is no way to tell from the document whether CTParser is non-conforming or exercising legitimate per-repo choice. §7 says per-repo label changes are a caller-only concern, which suggests the latter — but §2's table reads as prescriptive.

This matters concretely: the labels are the user-facing trigger surface, and anyone scripting across the org (or applying a label by muscle memory from a sibling repo) will find run ci cpu does nothing on CTParser and github-runner does nothing everywhere else.

Suggested fix — pick one

  1. Standardise on one vocabulary and converge the repos (either rename CTParser's two labels, or move the fleet to runner-based names).
  2. Explicitly allow both, adding a sentence to §2 that split-CI label names are a per-repo choice, and listing CTParser's github-runner/kkt-runner in the table as the alternative in use.

Option 2 is the smaller change; option 1 is better for cross-repo consistency. Flagging rather than deciding, since this is an org-wide call.


Also worth noting

CTParser.jl's Breakage.yml matrix listed only OptimalControl, but CTDirect.jl is also a genuine downstream consumer (real [deps] + [compat] CTParser = "0.8" entry, not merely a test [extras]). Fixed in control-toolbox/CTParser.jl#302. Not a handbook bug, but it suggests the §4 "which repo has which workflow" guidance would benefit from a note that breakage matrices should be re-derived from actual downstream [deps] rather than maintained by hand.

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentation

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions