Update verify tasks thresholds - #1949
Open
rsc3 wants to merge 11 commits into
Open
Conversation
|
Remember to squash merge! |
🔍Changelog Validation Results: |
🔍Version Validation Results: |
The threshold-update commit dropped the def calculate_md5 header, hash_md5 init, and file-open lines, leaving the read loop dangling. Every run failed with IndentationError before comparing any metric. Restore the 3 lines so md5 comparison runs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Remember to squash merge! |
🔍Changelog Validation Results: |
🔍Version Validation Results: |
The threshold-update commit dropped five metrics the scientific
Multiome test compares (fraction_confidently_mapped, fraction_unmapped,
fraction_nonnuclear, tss_enrichment_score,
fraction_of_high-quality_fragments_overlapping_peaks) and the
thresholds{k.lower()} normalization line. Without them those metrics
default to a 0 tolerance and fail on any nondeterministic drift, and
the mixed-case fraction_Q30 key never matched the lowercased lookup.
Re-add the five metrics (fraction_unmapped and overlapping_peaks
widened to 1e-5 / 1e-6 since their prior values still flaked), add a
number_of_cells tolerance (~10 cells), and restore the .lower()
normalization. Verified against submission 22618e75: all 14 previously
failing metrics now within threshold.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Remember to squash merge! |
🔍Version Validation Results: |
🔍Changelog Validation Results: |
|
Remember to squash merge! |
🔍Changelog Validation Results: |
🔍Version Validation Results: |
The CompareAtacLibraryMetrics thresholds were tuned to barely clear a single run's observed drift (several at 1.1-1.7x), which flakes intermittently as run-to-run nondeterminism varies. Recompute every metric that drifted in the latest scientific run to exactly 2x its observed relative drift (|test-truth|/truth). Still orders of magnitude below any real regression, so no loss of sensitivity; buys margin against flakes. Metrics decreased from prior values (number_of_cells, mean_raw_read_pairs_per_cell, median_high-quality_fragments_per_cell, overlapping_tss) were previously >2x and are brought in line with the uniform 2x-observed policy.
Capture the principle behind the CompareAtacLibraryMetrics threshold update: when a human accepts an observed nondeterminism drift, set the tolerance to double it, and note that metrics missing from a percentage-threshold table default to exact match.
…roadinstitute/warp into update-verify-tasks-thresholds
|
Remember to squash merge! |
🔍Changelog Validation Results: |
🔍Version Validation Results: |
FISS_MCP_RUNBOOK.md at repo root documents the path from a Terra submission URL to the exact failing task/metric via the fiss-mcp tools: the Test<Pipeline>.Verify subworkflow drill-down (empty parent logs), GCS log traversal, stdout-over-stderr, and stale-run checks. Scoped to Terra-based WDL pipeline tests only. Container-specific read quirks are marked as sandboxed-agent caveats that don't apply to a human with gsutil. Linked near the top of AGENTS.md.
|
Remember to squash merge! |
🔍Version Validation Results: |
🔍Changelog Validation Results: |
CompareH5adFilesGEX (called as CompareH5adFilesOptimus, and by Multiome/ PairedTag/SlideSeq/SlideTags) gates emptydrops_* column-sum drift at a hard 1%. emptydrops_PValue is an EmptyDrops Monte-Carlo p-value column and drifted 1.13% in a TestOptimus scientific run, tripping the 1% gate. Per the nondeterminism-threshold rule (AGENTS.md: double the human-accepted drift), raise the tolerance to 2x observed = 2.3%. Hoisted the constant to a single emptydrops_tol so the log strings can't drift from the value.
|
Remember to squash merge! |
🔍Changelog Validation Results: |
🔍Version Validation Results: |
|
Remember to squash merge! |
🔍Changelog Validation Results: |
🔍Version Validation Results: |
Collaborator
Author
|
GH run 33893240806 "Test PeakCalling" → completed/success on attempt 2 (updated 16:02:51Z, sha 1d46f60). The re-run drove Terra submission 383ff911… (today 15:24Z) → workflow Succeeded, 5/5 tasks Done. Attempt 1 had only 4 tasks with Verify blocked; this time CopyToTestResults ran Chain confirmed end to end:
Two takeaways:
One gotcha to ignore: the job endpoint 101089717375 still reads failure/attempt 1 — that's the old attempt's record; re-runs mint new per-attempt job ids. The run-level Nothing to fix in WARP. Branch left at 1d46f60, no files changed by me. Full trail in /workspace/JOURNAL.md. Fleet scan is now moot (re-run success already proves transient), but if you want blast-radius for the record, run this in your gcloud env: ✻ Cooked for 3m 20s |
|
Remember to squash merge! |
🔍Version Validation Results: |
🔍Changelog Validation Results: |
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.
…-determinism
Description
Give your PR a concise yet descriptive title.
Please explain the changes you made here.
Explain the motivation for making this change. What existing problem does the pull request solve?
Mention any issues fixed, addressed, or otherwise related to this pull request, including issue numbers or hard links for issues in other repos.
You can delete these instructions once you have written your PR description.
Checklist
If you can answer "yes" to the following items, please add a checkmark next to the appropriate checklist item(s) and notify our WARP team by tagging @broadinstitute/warp-admins in a comment on this PR.