Skip to content

PathoGFAIR: fix unresolvable fasttree pin + test schema regularization#1293

Merged
mvdbeek merged 3 commits into
galaxyproject:mainfrom
jmchilton:schema/pathogfair-fasttree-and-tests
Jul 16, 2026
Merged

PathoGFAIR: fix unresolvable fasttree pin + test schema regularization#1293
mvdbeek merged 3 commits into
galaxyproject:mainfrom
jmchilton:schema/pathogfair-fasttree-and-tests

Conversation

@jmchilton

@jmchilton jmchilton commented Jul 14, 2026

Copy link
Copy Markdown
Member

Supersedes #1162. Unblocks the PathoGFAIR aggregation workflow and carries its share of the test-schema regularization split out of #1286.

The fasttree pin

The workflow pinned fasttree 2.1.10+galaxy1, which is no longer installable — the Toolshed's only installable revisions are 2.1.10 (c7e73acfa3eb) and 2.1.10+galaxy3 (bc939965ce41). This produced two distinct failures:

This bumps all three fasttree steps to 2.1.10+galaxy3. planemo workflow_lint --iwc now exits 0.

What #1162 missed

#1162 updated content_id, tool_id, and changeset_revision, but left tool_version at 2.1.10+galaxy1 in all three fasttree steps. That inconsistency is fixed here.

The heatmap OOM (revealed once fasttree was fixed)

With the pin fixed, the workflow ran for the first time ever — 616 jobs succeeded, one errored: the ggplot2_heatmap step titled "Pathogeneic Genes Per Samples". It died in ~33s with empty stderr and no exit code — the signature of a container kill, not a clean R error.

Cause: the step is hard-coded to render at 70×70 cm @ 1000 dpi = a 27,559 × 27,559 px raster ≈ ~3 GB for the bitmap alone, on top of Galaxy + Postgres + docker on a ~7 GB runner. Near-certain OOM. This is pre-existing — it never surfaced before (in PRs or weeklies) because the fasttree pin blocked the workflow from ever reaching this step.

Fix: the physical size is legitimate (readable labels for many samples) and the vector PDF output already gives unlimited zoom for free, so the 1000-dpi raster is the only wasteful part. This exposes the PNG resolution as a workflow input heatmap_png_dpi (float), wired to the heatmap's dpi_output_dim. Width/height are untouched, so the vector PDF is unaffected.

Per review, the parameter defaults to 150 dpi (≈ 4,134 px², ~68 MB raster) so a plain run is memory-safe by default rather than relying on a test-only override; users who want a high-resolution raster can raise it, and the full-resolution vector PDF is always emitted regardless.

Test schema

Adds the class: Collection / collection_type annotations for this workflow's test file — the same mechanical change as #1290/#1291/#1292, held back from those because this workflow could not run at all.

Release

Folded into release 0.2 (fasttree update + the dpi parameter), since #1162's intermediate 0.1.1 never reached main.

…ularization

fasttree 2.1.10+galaxy1 is no longer installable from the toolshed (only
2.1.10 and 2.1.10+galaxy3 are), so the workflow could not be invoked and
lint errored. Supersedes galaxyproject#1162, which bumped content_id/tool_id/changeset
but left tool_version at +galaxy1 in all three fasttree steps.

Also carries the class:Collection / collection_type test annotations for
this workflow, split out of galaxyproject#1286.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "Pathogeneic Genes Per Samples" heatmap renders 70x70cm@1000dpi
(~27.5k px square PNG) and OOM-kills its container - the sole failure
once the fasttree pin is fixed and the workflow can finally run (616
jobs green, 1 error). Expose dpi as workflow input heatmap_png_dpi
(default 1000, width/height unchanged); test overrides to 150. Vector
PDF output is unaffected. Fold into release 0.2.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jmchilton

Copy link
Copy Markdown
Member Author

Note

Posted by Claude (AI assistant) on behalf of @jmchilton — not authored by them personally.

Why the latest commit adds a heatmap_png_dpi parameter

Once the fasttree pin was fixed, the workflow ran end-to-end for the first time ever: 616 jobs succeeded, exactly one errored — the ggplot2_heatmap step ("Pathogeneic Genes Per Samples"). It died in ~33s with empty stderr and no exit code, which is the signature of a killed container rather than a clean R error.

Root cause: that step is hard-coded to render at 70×70 cm @ 1000 dpi, i.e. a 27,559 × 27,559 px raster ≈ ~3 GB just for the bitmap — on top of Galaxy + Postgres + docker on a ~7 GB CI runner. Near-certain OOM. It had never surfaced before (in PRs or the weeklies) only because the fasttree pin blocked the workflow from ever reaching this step.

Why parameterize rather than just shrink it: the large physical size is legitimate — it keeps per-gene/per-sample labels readable — and the step already emits a vector PDF alongside the PNG, which gives unlimited zoom for free regardless of dpi. So the 1000-dpi raster is the only wasteful part; the physical dimensions and the PDF are worth keeping.

The change:

  • New workflow input heatmap_png_dpi (float, default 1000 — production behavior unchanged), wired to the heatmap's dpi_output_dim.
  • Width/height untouched → the vector PDF is unaffected.
  • The test overrides to 150 dpi (≈ 4,134 px², ~68 MB raster), comfortably within CI memory, so the workflow can go green without altering what production users get by default.

Folded into release 0.2 (fasttree update + this parameter), since #1162's intermediate 0.1.1 never reached main.

@mvdbeek

mvdbeek commented Jul 15, 2026

Copy link
Copy Markdown
Member
  • The test overrides to 150 dpi

i think this would be fine as a default value, could you make that change ?

@jmchilton

Copy link
Copy Markdown
Member Author

@mvdbeek Will do - Claude wanted to just change the value and not parameterize it but I didn't have the scientific domain experience/knowledge to be comfortable with that. Do you think 150 is fine enough as a default to drop this option (assuming this actually fixes the workflow).

@mvdbeek

mvdbeek commented Jul 15, 2026

Copy link
Copy Markdown
Member

I think 150 is pretty good, 300 is used for high-quality prints. 1000 sure seems like overkill. Since this is configurable I think it's in the interest of users and admins not to generate a 3GB image (by default).

@jmchilton

Copy link
Copy Markdown
Member Author

Since this is configurable

Well it is only configurable at the workflow level because I let Claude mangle this workflow 😅. Definitely changing the default though either way.

Make the workflow safe by default instead of relying on a test-only
override: set the heatmap_png_dpi parameter default to 150 (was 1000)
so a plain run no longer OOMs on the ~27.5k px square PNG. Drops the
now-redundant dpi override from the test. Vector PDF unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mvdbeek
mvdbeek merged commit 1a4953d into galaxyproject:main Jul 16, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants