feat(sklearn): refuse the label column as an input column - #8012
Open
kz930 wants to merge 1 commit into
Open
Conversation
The Target Attribute could also be named as a Text Attribute, and the Ground Truth Attribute Column as a Selected Feature. The first ends the run from inside scikit-learn naming neither field, and the second does not fail at all: the estimator is handed the answer as an input. Both are refused while the workflow is being written, beside the Count Vectorizer refusal the shared base already carries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8012 +/- ##
============================================
- Coverage 93.29% 93.29% -0.01%
+ Complexity 4678 4677 -1
============================================
Files 1179 1179
Lines 47713 47722 +9
Branches 5314 5316 +2
============================================
+ Hits 44515 44520 +5
- Misses 1732 1734 +2
- Partials 1466 1468 +2
*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:
|
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🟢 | bs=10 sw=10 sl=64 | 656 | 0.401 | 14,852/17,482/17,482 us | 🟢 -27.5% / 🔴 +33.3% |
| 🔴 | bs=100 sw=10 sl=64 | 1,384 | 0.844 | 72,990/79,785/79,785 us | 🟢 -23.8% / 🟢 -21.9% |
| 🟢 | bs=1000 sw=10 sl=64 | 1,552 | 0.947 | 641,148/694,608/694,608 us | 🟢 -6.2% / 🟢 +29.5% |
Baseline details
Latest main 9fe0657 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 656 tuples/sec | 643 tuples/sec | 894.86 tuples/sec | +2.0% | -26.7% |
| bs=10 sw=10 sl=64 | MB/s | 0.401 MB/s | 0.392 MB/s | 0.546 MB/s | +2.3% | -26.6% |
| bs=10 sw=10 sl=64 | p50 | 14,852 us | 14,519 us | 11,139 us | +2.3% | +33.3% |
| bs=10 sw=10 sl=64 | p95 | 17,482 us | 24,118 us | 13,843 us | -27.5% | +26.3% |
| bs=10 sw=10 sl=64 | p99 | 17,482 us | 24,118 us | 16,839 us | -27.5% | +3.8% |
| bs=100 sw=10 sl=64 | throughput | 1,384 tuples/sec | 1,376 tuples/sec | 1,166 tuples/sec | +0.6% | +18.7% |
| bs=100 sw=10 sl=64 | MB/s | 0.844 MB/s | 0.84 MB/s | 0.711 MB/s | +0.5% | +18.6% |
| bs=100 sw=10 sl=64 | p50 | 72,990 us | 69,437 us | 87,509 us | +5.1% | -16.6% |
| bs=100 sw=10 sl=64 | p95 | 79,785 us | 104,660 us | 93,675 us | -23.8% | -14.8% |
| bs=100 sw=10 sl=64 | p99 | 79,785 us | 104,660 us | 102,153 us | -23.8% | -21.9% |
| bs=1000 sw=10 sl=64 | throughput | 1,552 tuples/sec | 1,564 tuples/sec | 1,198 tuples/sec | -0.8% | +29.5% |
| bs=1000 sw=10 sl=64 | MB/s | 0.947 MB/s | 0.955 MB/s | 0.731 MB/s | -0.8% | +29.5% |
| bs=1000 sw=10 sl=64 | p50 | 641,148 us | 635,283 us | 859,766 us | +0.9% | -25.4% |
| bs=1000 sw=10 sl=64 | p95 | 694,608 us | 740,491 us | 902,897 us | -6.2% | -23.1% |
| bs=1000 sw=10 sl=64 | p99 | 694,608 us | 740,491 us | 937,957 us | -6.2% | -25.9% |
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,304.79,200,128000,656,0.401,14851.65,17481.86,17481.86
1,100,10,64,20,1445.54,2000,1280000,1384,0.844,72989.51,79785.31,79785.31
2,1000,10,64,20,12883.49,20000,12800000,1552,0.947,641147.52,694608.26,694608.26
Contributor
Author
|
/request-review @carloea2 @aglinxinyuan |
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
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.
What changes were proposed in this PR?
Two families of sklearn operators let the column already chosen as the label be chosen a second time as an input column, and neither said anything while the workflow was being written.
Target Attributenames the label andText Attributenames the columnsCount Vectorizertokenizes, on the fifty-one operators of the Sklearn and Sklearn Training groups. The generated code drops the target before the text pipeline reads its columns, so naming it there asks the pipeline for a column that is no longer present and the run ends withValueError: A given column is not a column of the dataframe, which names neither field. Failing is right, since vectorizing the label would train a model on the answer, so the pairing is refused at compile time instead, in the samegetOutputSchemasthat already refusesCount Vectorizeron the two Gaussian Naive Bayes operators. Only while the vectorizer is on: with it off nothing readstextand the panel hides it, so a value left behind by an earlier configuration must not report the operator invalid.Ground Truth Attribute Columnnames the label andSelected Featuresnames the columns handed tofit, on the four advanced trainers. Here nothing failed at all. The features go tofitas they are named, so the ground truth among them is the answer given to the estimator as an input, and the model scores far better than what it learned deserves. On two hundred rows whose label is random noise, a KNN classifier reaches 0.685 train accuracy on the two real features and 0.995 once the label is included, and nothing in the run says why.selectedFeaturesdefaults to null, so the check reads it through anOptionrather than assuming a list is there.Both messages name the column and both fields, and say what to change.
Any related issues, documentation, discussions?
Closes #8010.
How was this PR tested?
SklearnModelOpDescSpecgains three cases: the target named as a text column is refused, text columns that are not the target are let through, and a stale text column is left alone whileCount Vectorizeris off.SklearnMLOperatorDescriptorSpecgains two: the ground truth named as a feature is refused, and features that do not include it are let through. Both files already owned the assertions on thegetOutputSchemasthese checks live in.WorkflowOperator/testpasses: 2381 tests.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)