Skip to content

fix(sklearn): rename SVR shrinking parameter - #7973

Open
VuMartin wants to merge 1 commit into
apache:mainfrom
VuMartin:fix/svm-regressor-shrinking-parameter
Open

fix(sklearn): rename SVR shrinking parameter#7973
VuMartin wants to merge 1 commit into
apache:mainfrom
VuMartin:fix/svm-regressor-shrinking-parameter

Conversation

@VuMartin

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Renamed the probability parameter to shrinking in SklearnAdvancedSVRParameters.

The generated Python parameter was already correctly set to shrinking; only the displayed parameter name was incorrect.

Before:
Screenshot 2026-08-25 at 1 12 24 AM
After:
Screenshot 2026-08-25 at 1 35 53 AM

Note: This renames the enum constant from probability to shrinking. Existing workflows saved with probability will need to be updated.

Any related issues, documentation, discussions?

Fixes #7949

How was this PR tested?

Tested the Advanced SVM Regressor Trainer with shrinking = true and verified that the resulting parameters show shrinking = True.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: ChatGPT (5.5 GPT)

@Yicong-Huang Yicong-Huang added the release/v1.2 back porting to release/v1.2 label Aug 25, 2026
@github-actions
github-actions Bot requested a review from xuang7 August 25, 2026 10:22
@github-actions

Copy link
Copy Markdown
Contributor

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. release/* labels drive the post-merge backport, so add or remove one to change where this fix lands.

Release branch Analysis
release/v1.2 Change detected on this branch — label added; this fix is queued to backport here. Requested review from @xuang7.

Auto-label run.

@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @Yicong-Huang
    You can notify them by mentioning @Yicong-Huang in a comment.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 1 better · 🔴 6 worse · ⚪ 8 noise (<±5%) · 0 without baseline

Compared against main 2c2c0e5 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🟢 bs=10 sw=10 sl=64 434 0.265 22,382/34,535/34,535 us 🟢 -8.4% / 🔴 +145.5%
🔴 bs=100 sw=10 sl=64 905 0.552 105,925/146,313/146,313 us 🔴 +11.0% / 🔴 +51.6%
🔴 bs=1000 sw=10 sl=64 1,090 0.665 914,074/1,046,122/1,046,122 us 🔴 +11.7% / 🔴 +12.5%
Baseline details

Latest main 2c2c0e5 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 434 tuples/sec 425 tuples/sec 863.81 tuples/sec +2.1% -49.8%
bs=10 sw=10 sl=64 MB/s 0.265 MB/s 0.259 MB/s 0.527 MB/s +2.3% -49.7%
bs=10 sw=10 sl=64 p50 22,382 us 24,443 us 11,488 us -8.4% +94.8%
bs=10 sw=10 sl=64 p95 34,535 us 33,438 us 14,067 us +3.3% +145.5%
bs=10 sw=10 sl=64 p99 34,535 us 33,438 us 17,591 us +3.3% +96.3%
bs=100 sw=10 sl=64 throughput 905 tuples/sec 968 tuples/sec 1,127 tuples/sec -6.5% -19.7%
bs=100 sw=10 sl=64 MB/s 0.552 MB/s 0.591 MB/s 0.688 MB/s -6.6% -19.8%
bs=100 sw=10 sl=64 p50 105,925 us 102,257 us 89,893 us +3.6% +17.8%
bs=100 sw=10 sl=64 p95 146,313 us 131,868 us 96,506 us +11.0% +51.6%
bs=100 sw=10 sl=64 p99 146,313 us 131,868 us 104,915 us +11.0% +39.5%
bs=1000 sw=10 sl=64 throughput 1,090 tuples/sec 1,105 tuples/sec 1,156 tuples/sec -1.4% -5.7%
bs=1000 sw=10 sl=64 MB/s 0.665 MB/s 0.675 MB/s 0.705 MB/s -1.5% -5.7%
bs=1000 sw=10 sl=64 p50 914,074 us 909,827 us 886,448 us +0.5% +3.1%
bs=1000 sw=10 sl=64 p95 1,046,122 us 936,542 us 930,093 us +11.7% +12.5%
bs=1000 sw=10 sl=64 p99 1,046,122 us 936,542 us 963,104 us +11.7% +8.6%
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,460.91,200,128000,434,0.265,22381.85,34534.50,34534.50
1,100,10,64,20,2210.33,2000,1280000,905,0.552,105925.45,146313.44,146313.44
2,1000,10,64,20,18347.87,20000,12800000,1090,0.665,914074.17,1046122.42,1046122.42

@codecov-commenter

codecov-commenter commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.82%. Comparing base (3198595) to head (3857ad6).
⚠️ Report is 22 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7973      +/-   ##
============================================
+ Coverage     92.79%   92.82%   +0.03%     
- Complexity     4616     4735     +119     
============================================
  Files          1174     1175       +1     
  Lines         47453    48071     +618     
  Branches       5304     5409     +105     
============================================
+ Hits          44034    44624     +590     
- Misses         1899     1902       +3     
- Partials       1520     1545      +25     
Flag Coverage Δ *Carryforward flag
access-control-service 81.00% <ø> (ø)
agent-service 98.62% <ø> (ø) Carriedforward from 3198595
amber 89.36% <100.00%> (+0.23%) ⬆️
computing-unit-managing-service 73.67% <ø> (ø)
config-service 86.73% <ø> (ø)
file-service 86.05% <ø> (+1.56%) ⬆️
frontend 95.03% <ø> (ø) Carriedforward from 3198595
notebook-migration-service 82.36% <ø> (+3.23%) ⬆️
pyamber 97.57% <ø> (ø) Carriedforward from 3198595
workflow-compiling-service 77.19% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xuang7 xuang7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@xuang7 xuang7 added release/v1.2 back porting to release/v1.2 and removed release/v1.2 back porting to release/v1.2 labels Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common fix release/v1.2 back porting to release/v1.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The SVM Regressor trainer offers a hyperparameter named probability, which sets shrinking instead

4 participants