Benchmark scikit-learn PR #34793 (Ridge only) - #31
Conversation
|
Comparing |
|
intel-gnr: scikit-learn Dashboard: https://pr-31-compare-intel-gnr.sklbench-pr-comparison.pages.dev/pr_comparison Full run: https://github.com/probabl-ai/scikit-learn-benchmarks/actions/runs/32993244861 |
|
intel-laptop: scikit-learn Dashboard: https://pr-31-compare-intel-laptop.sklbench-pr-comparison.pages.dev/pr_comparison Full run: https://github.com/probabl-ai/scikit-learn-benchmarks/actions/runs/32993244861 |
Confirmed live on PR #31: the "Validate config" step used -e reporting (and run.sh's own preflight used -e default), but implementations_for_pixi_env() rejects any PIXI_ENVIRONMENT_NAME outside its real-env allowlist - so this preflight was broken for any config calling it (all_models.py and anything derived from it, e.g. this PR's own all_models_ridge_only.py), unconditionally, regardless of which real environment would go on to run. - run.sh: drop the recently-added --validate-only preflight block and its now-unused extract_configs helper. - scripts/classify_bench_session.py: drop the now-dead special-case for run.sh's removed --validate-only header line. - .github/workflows/pr-comparison.yml: reuse run.sh directly (one invocation with two env@owner:ref entries) instead of duplicating setup_sklearn_ref.sh + sklbench calls per side - matches CONTRIBUTING.md's documented PR-vs-base comparison pattern exactly, and collapses two log files/classify calls into one. Keep a config preflight, but scoped to this workflow with a real, always-installable env (sklearn-pypi) instead of a broken one - still worth catching a typo'd config before a many-minutes sklearn-dev clone+build. - scripts/resolve_sklearn_pr.py: output the fork's `owner` (matching run.sh's env@owner:ref shorthand) instead of a full clone_url, and validate the fork's repo is actually named "scikit-learn" (what that shorthand assumes) rather than silently mishandling a differently-named fork. - tests/test_config_loading.py: register all_models_ridge_only.py in ENV_SENSITIVE_CONFIGS alongside the other all_models.py-derived configs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
c00a46d to
65ba120
Compare
…ght (#32) Confirmed live on PR #31: the "Validate config" step used -e reporting (and run.sh's own preflight used -e default), but implementations_for_pixi_env() rejects any PIXI_ENVIRONMENT_NAME outside its real-env allowlist - so this preflight was broken for any config calling it (all_models.py and anything derived from it, e.g. this PR's own all_models_ridge_only.py), unconditionally, regardless of which real environment would go on to run. - run.sh: drop the recently-added --validate-only preflight block and its now-unused extract_configs helper. - scripts/classify_bench_session.py: drop the now-dead special-case for run.sh's removed --validate-only header line. - .github/workflows/pr-comparison.yml: reuse run.sh directly (one invocation with two env@owner:ref entries) instead of duplicating setup_sklearn_ref.sh + sklbench calls per side - matches CONTRIBUTING.md's documented PR-vs-base comparison pattern exactly, and collapses two log files/classify calls into one. Keep a config preflight, but scoped to this workflow with a real, always-installable env (sklearn-pypi) instead of a broken one - still worth catching a typo'd config before a many-minutes sklearn-dev clone+build. - scripts/resolve_sklearn_pr.py: output the fork's `owner` (matching run.sh's env@owner:ref shorthand) instead of a full clone_url, and validate the fork's repo is actually named "scikit-learn" (what that shorthand assumes) rather than silently mishandling a differently-named fork. - tests/test_config_loading.py: register all_models_ridge_only.py in ENV_SENSITIVE_CONFIGS alongside the other all_models.py-derived configs. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
For benchmarking scikit-learn PR #34793 (ridge/optim_cholesky) via the sklbench-compare flow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
65ba120 to
28022ea
Compare
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The site has no index.html (only pr_comparison.html), so the bare Cloudflare Pages domain 404s - confirmed live on PR #31's intel-laptop dashboard link. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The site has no index.html (only pr_comparison.html), so the bare Cloudflare Pages domain 404s - confirmed live on PR #31's intel-laptop dashboard link. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Dashboard preview: https://pr-31.sklbench-dashboard-preview.pages.dev Built from |
Benchmarks for PR scikit-learn/scikit-learn#34793
The PR doesn't touch the prediction path, but the table/benchmarks still show speed-ups/slow-downs; seems this is really noisy, to be investigated/fixed later.
For this PR, only the fit time is relevant.