Repository navigation
feat(workflow-operator): volcano plot significance cut-offs - #9041
eugenegujing wants to merge 3 commits into
Conversation
Add optional Significance threshold and Effect-size threshold properties that draw dashed cut-off lines at -log10(threshold) and at plus and minus the effect-size threshold. Add a Color by significance toggle that, with both thresholds set, colors points Up, Down or Not significant instead of by effect size. Bound both thresholds in the JSON schema and ignore a non-positive, non-finite or above-one value instead of drawing it.
Automated Reviewer SuggestionsBased on the
|
…ignificance-thresholds
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9041 +/- ##
=========================================
Coverage 91.87% 91.87%
Complexity 5049 5049
=========================================
Files 1265 1265
Lines 54915 54913 -2
Branches 6940 6940
=========================================
- Hits 50453 50452 -1
+ Misses 2769 2768 -1
Partials 1693 1693
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 378 | 0.23 | 26,351/35,663/35,663 us | 🟢 -60.5% / 🔴 +150.6% |
| 🔴 | bs=100 sw=10 sl=64 | 835 | 0.51 | 115,024/156,302/156,302 us | 🔴 +15.1% / 🔴 +62.0% |
| ⚪ | bs=1000 sw=10 sl=64 | 982 | 0.599 | 1,017,149/1,077,295/1,077,295 us | ⚪ within ±5% / 🔴 +18.4% |
Baseline details
Latest main 98c6337 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 378 tuples/sec | 379 tuples/sec | 879.16 tuples/sec | -0.3% | -57.0% |
| bs=10 sw=10 sl=64 | MB/s | 0.23 MB/s | 0.231 MB/s | 0.537 MB/s | -0.4% | -57.1% |
| bs=10 sw=10 sl=64 | p50 | 26,351 us | 24,156 us | 11,504 us | +9.1% | +129.1% |
| bs=10 sw=10 sl=64 | p95 | 35,663 us | 90,374 us | 14,234 us | -60.5% | +150.6% |
| bs=10 sw=10 sl=64 | p99 | 35,663 us | 90,374 us | 17,338 us | -60.5% | +105.7% |
| bs=100 sw=10 sl=64 | throughput | 835 tuples/sec | 886 tuples/sec | 1,147 tuples/sec | -5.8% | -27.2% |
| bs=100 sw=10 sl=64 | MB/s | 0.51 MB/s | 0.54 MB/s | 0.7 MB/s | -5.6% | -27.2% |
| bs=100 sw=10 sl=64 | p50 | 115,024 us | 111,137 us | 88,987 us | +3.5% | +29.3% |
| bs=100 sw=10 sl=64 | p95 | 156,302 us | 135,787 us | 96,473 us | +15.1% | +62.0% |
| bs=100 sw=10 sl=64 | p99 | 156,302 us | 135,787 us | 102,791 us | +15.1% | +52.1% |
| bs=1000 sw=10 sl=64 | throughput | 982 tuples/sec | 999 tuples/sec | 1,182 tuples/sec | -1.7% | -16.9% |
| bs=1000 sw=10 sl=64 | MB/s | 0.599 MB/s | 0.61 MB/s | 0.721 MB/s | -1.8% | -17.0% |
| bs=1000 sw=10 sl=64 | p50 | 1,017,149 us | 999,973 us | 872,751 us | +1.7% | +16.5% |
| bs=1000 sw=10 sl=64 | p95 | 1,077,295 us | 1,039,866 us | 910,151 us | +3.6% | +18.4% |
| bs=1000 sw=10 sl=64 | p99 | 1,077,295 us | 1,039,866 us | 941,479 us | +3.6% | +14.4% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,529.67,200,128000,378,0.230,26351.19,35662.75,35662.75
1,100,10,64,20,2393.99,2000,1280000,835,0.510,115023.87,156301.77,156301.77
2,1000,10,64,20,20374.77,20000,12800000,982,0.599,1017148.93,1077294.56,1077294.56
kz930
left a comment
There was a problem hiding this comment.
LGTM. The schema bounds plus the runtime filter cover both the property panel and hand-edited JSON.
What changes were proposed in this PR?
The Volcano Plot draws a plain scatter of effect size vs
-log10(p), colored by effect size, which is already the x axis. Nothing shows where the significance bar is or which points clear it. This PR adds three optional properties, all off by default, so saved workflows render as before.0 < x <= 1-log10(threshold)> 0+thresholdand-thresholdUp/Down/Not significantwith a legend instead of the continuousRdBuscaleCategories use strict comparisons on the selected p-value column:
Up=p < sigandeffect > eff,Down=p < sigandeffect < -eff, everything elseNot significant. The category is written to a column namedSignificance(also the legend title); an input column of that name would be shadowed inside the operator only. Colors are ColorBrewer RdBu end points#B2182B/#2166ACplus grey#9E9E9E.Validation: the JSON schema bounds above stop bad values in the property panel. For hand-edited JSON the operator also ignores a threshold it cannot draw (NaN, infinite, <= 0, or a p cut-off > 1) and falls back to the continuous coloring if the toggle is on but a threshold is missing. A cut-off of exactly 1 is allowed and draws its line at y = 0. If the cut-off's
-log10is beyond the data range the line may fall outside the auto-ranged axis; extending the range is left for a follow-up.Since #8504 merged, the operator also exports a standalone script through
generateStandaloneCode(). This PR adds the cut-off lines, theSignificancecolumn and the three-color map there too, from the same helpers where possible: the color map and category order are one shared literal, the threshold lines come from the same builder, and only the column names differ (Python literals instead of decoded strings). A spec checks that both paths make the same line and coloring decisions for eight threshold shapes, so the exported figure cannot drift from the engine's.Independent of the label-fix (#9037) and hover-column (#9038) PRs; all three touch neighbouring lines of
VolcanoPlotOpDesc.scala, so whichever lands second needs a small rebase.Figures below are the operator's generated Python from
mainand from this branch, run on the 430-gene example workflow from the issue, with thresholds 0.05 and 1.Any related issues, documentation, discussions?
Closes #9039
PR #8504 (merged) added the
generateStandaloneCode()export path; the mirroring described above is the follow-up it needed.How was this PR tested?
New tests in
VolcanoPlotOpDescSpec, written before the source change:np.selectcolumn, exact color map and category order, nocolor_continuous_scaleInfinity,-Infinity,NaNfrom JSON never reach the Python1e-300,Double.MIN_VALUEand1e21in every site they appearexclusiveMinimum/maximumon the two thresholdsgenerateStandaloneCode: defaults draw no line and keep the continuous coloring; thresholds with the toggle off draw the dashed lines only; the toggle on colorsUp/Down/Not significantwith the column names as literals; a cross-path test checks that the exported script makes the same cut-off and coloring decisions as the engine for eight threshold shapes (unset, one side only, zero, p cut-off above 1, non-finite, exactly 1); a saved workflow renders as a default-constructed operatorSmoke run outside Texera (pytexera stubbed, pandas + plotly 5.24.1) on the 430-gene mock table, 420 rows after the operator drops zero and missing p-values:
Up35,Down35,Not significant350"Infinity"/"NaN"from JSONNot significant420Up41,Down38,Not significant341The exponent-form dumps pass
python -m py_compile. The exported script for the default, lines-only and colored configurations parses withast.parse.Was this PR authored or co-authored using generative AI tooling?
Yes. Generated-by: Claude Code (claude-fable-5-1). Co-authored with Claude; the author reviewed the change before submission.