test(workflow-operator): add unit tests for SklearnModelOpDesc - #6665
Conversation
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6665 +/- ##
=========================================
Coverage 74.65% 74.65%
Complexity 3438 3438
=========================================
Files 1160 1160
Lines 45793 45793
Branches 5069 5069
=========================================
Hits 34189 34189
Misses 9953 9953
Partials 1651 1651
*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 | 443 | 0.271 | 21,919/28,549/28,549 us | 🔴 +15.5% / 🔴 +82.6% |
| 🔴 | bs=100 sw=10 sl=64 | 921 | 0.562 | 105,795/157,352/157,352 us | 🔴 +23.2% / 🔴 +48.5% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,090 | 0.665 | 917,304/948,914/948,914 us | ⚪ within ±5% / 🟢 -9.5% |
Baseline details
Latest main 0467c76 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 443 tuples/sec | 492 tuples/sec | 767.23 tuples/sec | -10.0% | -42.3% |
| bs=10 sw=10 sl=64 | MB/s | 0.271 MB/s | 0.301 MB/s | 0.468 MB/s | -10.0% | -42.1% |
| bs=10 sw=10 sl=64 | p50 | 21,919 us | 18,977 us | 12,705 us | +15.5% | +72.5% |
| bs=10 sw=10 sl=64 | p95 | 28,549 us | 29,918 us | 15,633 us | -4.6% | +82.6% |
| bs=10 sw=10 sl=64 | p99 | 28,549 us | 29,918 us | 20,214 us | -4.6% | +41.2% |
| bs=100 sw=10 sl=64 | throughput | 921 tuples/sec | 974 tuples/sec | 1,001 tuples/sec | -5.4% | -8.0% |
| bs=100 sw=10 sl=64 | MB/s | 0.562 MB/s | 0.595 MB/s | 0.611 MB/s | -5.5% | -8.0% |
| bs=100 sw=10 sl=64 | p50 | 105,795 us | 99,844 us | 99,949 us | +6.0% | +5.8% |
| bs=100 sw=10 sl=64 | p95 | 157,352 us | 127,727 us | 105,996 us | +23.2% | +48.5% |
| bs=100 sw=10 sl=64 | p99 | 157,352 us | 127,727 us | 117,999 us | +23.2% | +33.4% |
| bs=1000 sw=10 sl=64 | throughput | 1,090 tuples/sec | 1,104 tuples/sec | 1,033 tuples/sec | -1.3% | +5.5% |
| bs=1000 sw=10 sl=64 | MB/s | 0.665 MB/s | 0.674 MB/s | 0.631 MB/s | -1.3% | +5.5% |
| bs=1000 sw=10 sl=64 | p50 | 917,304 us | 911,050 us | 974,571 us | +0.7% | -5.9% |
| bs=1000 sw=10 sl=64 | p95 | 948,914 us | 944,008 us | 1,016,467 us | +0.5% | -6.6% |
| bs=1000 sw=10 sl=64 | p99 | 948,914 us | 944,008 us | 1,048,798 us | +0.5% | -9.5% |
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,451.13,200,128000,443,0.271,21919.05,28549.22,28549.22
1,100,10,64,20,2172.26,2000,1280000,921,0.562,105795.00,157352.09,157352.09
2,1000,10,64,20,18349.09,20000,12800000,1090,0.665,917304.47,948913.67,948913.67There was a problem hiding this comment.
Pull request overview
Adds ScalaTest unit coverage for SklearnModelOpDesc (a shared abstract sklearn operator descriptor in common/workflow-operator) to lock in its default flag values and its fixed output-schema contract, as requested in #6664.
Changes:
- Introduces
SklearnModelOpDescSpecwith a minimal concrete subclass to instantiate the abstract descriptor. - Asserts
countVectorizerandtfidfTransformerdefault tofalse. - Verifies
getOutputSchemasalways returns exactly one output keyed by the output port id, withmodel_name: STRINGandmodel: BINARY, independent of input schemas.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What changes were proposed in this PR?
Any related issues, documentation, discussions?
Closes: #6664
How was this PR tested?
sbt "WorkflowOperator/testOnly *SklearnModelOpDescSpec", expect all 4 tests passing.Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude Opus 4.8 in compliance with ASF