refactor(workflow-operator): rename PythonCodegenBase to HuggingFaceCodegenBase for clarity - #7143
Conversation
Automated Reviewer SuggestionsBased on the
|
|
/request-review @xuang7 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7143 +/- ##
============================================
- Coverage 79.59% 79.58% -0.01%
Complexity 3838 3838
============================================
Files 1159 1159
Lines 46122 46121 -1
Branches 5127 5127
============================================
- Hits 36709 36707 -2
Misses 7787 7787
- Partials 1626 1627 +1
*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 | 388 | 0.237 | 25,134/31,512/31,512 us | 🔴 +16.8% / 🔴 +99.6% |
| 🟢 | bs=100 sw=10 sl=64 | 785 | 0.479 | 124,966/154,868/154,868 us | 🟢 -7.2% / 🔴 +40.4% |
| ⚪ | bs=1000 sw=10 sl=64 | 910 | 0.555 | 1,097,555/1,155,595/1,155,595 us | ⚪ within ±5% / 🔴 +9.5% |
Baseline details
Latest main 084309c from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 388 tuples/sec | 452 tuples/sec | 771.62 tuples/sec | -14.2% | -49.7% |
| bs=10 sw=10 sl=64 | MB/s | 0.237 MB/s | 0.276 MB/s | 0.471 MB/s | -14.1% | -49.7% |
| bs=10 sw=10 sl=64 | p50 | 25,134 us | 21,519 us | 12,626 us | +16.8% | +99.1% |
| bs=10 sw=10 sl=64 | p95 | 31,512 us | 30,433 us | 15,786 us | +3.5% | +99.6% |
| bs=10 sw=10 sl=64 | p99 | 31,512 us | 30,433 us | 19,344 us | +3.5% | +62.9% |
| bs=100 sw=10 sl=64 | throughput | 785 tuples/sec | 770 tuples/sec | 971.56 tuples/sec | +1.9% | -19.2% |
| bs=100 sw=10 sl=64 | MB/s | 0.479 MB/s | 0.47 MB/s | 0.593 MB/s | +1.9% | -19.2% |
| bs=100 sw=10 sl=64 | p50 | 124,966 us | 130,088 us | 103,463 us | -3.9% | +20.8% |
| bs=100 sw=10 sl=64 | p95 | 154,868 us | 166,908 us | 110,296 us | -7.2% | +40.4% |
| bs=100 sw=10 sl=64 | p99 | 154,868 us | 166,908 us | 118,690 us | -7.2% | +30.5% |
| bs=1000 sw=10 sl=64 | throughput | 910 tuples/sec | 928 tuples/sec | 1,001 tuples/sec | -1.9% | -9.1% |
| bs=1000 sw=10 sl=64 | MB/s | 0.555 MB/s | 0.566 MB/s | 0.611 MB/s | -1.9% | -9.2% |
| bs=1000 sw=10 sl=64 | p50 | 1,097,555 us | 1,075,976 us | 1,008,988 us | +2.0% | +8.8% |
| bs=1000 sw=10 sl=64 | p95 | 1,155,595 us | 1,114,864 us | 1,055,260 us | +3.7% | +9.5% |
| bs=1000 sw=10 sl=64 | p99 | 1,155,595 us | 1,114,864 us | 1,074,689 us | +3.7% | +7.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,515.79,200,128000,388,0.237,25133.66,31511.98,31511.98
1,100,10,64,20,2547.10,2000,1280000,785,0.479,124965.74,154868.19,154868.19
2,1000,10,64,20,21975.08,20000,12800000,910,0.555,1097555.00,1155595.05,1155595.05
What changes were proposed in this PR?
Pure internal rename:
PythonCodegenBase->HuggingFaceCodegenBase(object, file, and spec).Despite the generic name, this object is HF-specific, it lives in
.../huggingFace/codegen/, is used only there, and is the shared base of the Hugging Face per-taskTaskCodegens. The old name led a reviewer to ask whether the HF logic belonged elsewhere (comment on #6972); the new name matches the sibling codegens andHuggingFaceInferenceOpDesc.Also updated the references in
HuggingFaceInferenceOpDesc.scala,ImageTaskCodegen.scala, andTaskCodegen.scala. Behavior-neutral: no change to the generated Python, workflow JSON, or any public API; git tracks both files as renames.Any related issues, documentation, discussions?
Closes #7140
Follow-up to the review discussion on #6972.
How was this PR tested?
No new tests as this is a rename, and existing coverage confirms it's behavior-neutral.
The full HF package passes (124 tests), including
PythonCodeRawInvalidTextSpec, whichpy-compiles the generated Python of all 117 operators — confirming the generated output is
unchanged. scalafmt clean.
Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude Opus 4.7 in compliance with ASF.