Skip to content

ci/triton resilience and eager ml tools tests - #1609

Open
NJManganelli wants to merge 7 commits into
scikit-hep:masterfrom
NJManganelli:ci/triton-resilience-and-eager-ml-tools-tests
Open

ci/triton resilience and eager ml tools tests#1609
NJManganelli wants to merge 7 commits into
scikit-hep:masterfrom
NJManganelli:ci/triton-resilience-and-eager-ml-tools-tests

Conversation

@NJManganelli

Copy link
Copy Markdown
Collaborator

🧑‍🦱 Human text follows 👨‍🦲

This tries to apply some fixes for the triton client failures seen in some recent CI runs: mostly intermittent failure to reach the image server, with the remedy being 1) 4 retries spaced exponentially apart (Claude originally tried 15s gaps x4, I prefer this exponentiating one) and 2) splitting tests to run both eagerly and in dask-awkward mode (Claude originally just removed these entirely from non-dask tagged tests... but that's not quite right, no brushing it under the rug, claude-bot)

Some long comments... but this one might be worthwhile keeping

@yimuchen if you want to throw a veto on anything here, you're welcome to.

🤖 AI text follows 🤖
Squash-merge collapses the revert pair, so the noise doesn't survive.

Two things worth carrying into the PR description:

  • The module-level importorskip fix is the load-bearing one. Old module collected 0 tests without dask_awkward; new one collects 8. That's a coverage hole, not a cosmetic change.
  • test_tensorflow_* skips in CI too, not just locally — coffea's create-args list only pytorch-cpu and py-xgboost. The tf wrapper is untested everywhere. Separate issue.

Nick Manganelli added 7 commits July 26, 2026 13:56
test_triton is the only test touching the inference server and it is marked
dask_client. The "without dask Client" job selects -m "not dask_client", so the
server it starts is never contacted. Pulling the image there costs a multi-
gigabyte download on six matrix entries and makes the job fail whenever nvcr.io
is unreachable, which then cancels the rest of the matrix.

Assisted-by: Claude Opus 4.7 (1M context)
Pulls of the triton image intermittently fail with a connection timeout to
nvcr.io, which failed the whole job before any test ran. Retry the start three
times with a widening delay, and on exhaustion record TRITON_UNAVAILABLE so
test_triton skips instead.

The skip is keyed on that flag rather than on server reachability, so it applies
only when the image could not be fetched. A server that starts and then
misbehaves still fails the test.

Assisted-by: Claude Opus 4.7 (1M context)
Three attempts 15s apart cover only a 45-second outage. Five attempts spaced
15s, 1m, 5m and 20m ride out a transient registry problem while bounding a
sustained one at roughly 26 minutes of waiting.

Assisted-by: Claude Opus 4.7 (1M context)
Every ml_tools test mixed eager and dask_awkward assertions in one dask_client
marked function, so under -m "not dask_client" the eager path of the triton,
torch, tensorflow and xgboost wrappers had no coverage at all — the path a user
without dask takes, and the default configuration since dask became optional.

Split each test in two: an unmarked eager test asserting the wrapper runs and
returns the expected lengths, and the dask_client test that compares the two
backends and checks necessary_columns. Wrapper construction moves into shared
factories so neither copy duplicates it.

dask_awkward is now imported per test instead of at module scope. Previously its
absence skipped the whole module, including tests that never touch it.

Assisted-by: Claude Opus 4.7 (1M context)
Both the with- and without-dask-client jobs run triton tests now, so both need
the same tolerance for a registry outage rather than only the one that had it.

Assisted-by: Claude Opus 4.7 (1M context)
_ak for the plain awkward path, _dak for the dask_awkward one, rather than
leaving the dask variant unsuffixed. The pair is now symmetric and the array
type under test is visible in the name.

The _ak tests stay awkward-only. Coffea's virtual mode is a property of how a
factory materializes buffers from a file, not something these directly built
arrays can carry, and numpy_call_wrapper dispatches on dask_awkward alone, so a
virtual array would take the same path as an eager one.

Assisted-by: Claude Opus 4.7 (1M context)
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.

1 participant