Skip to content

Add plan to re-split bench_cfg.py with a clean API break#923

Open
blooop wants to merge 2 commits into
mainfrom
claude/plan-reenable-pr-688-YAzSR
Open

Add plan to re-split bench_cfg.py with a clean API break#923
blooop wants to merge 2 commits into
mainfrom
claude/plan-reenable-pr-688-YAzSR

Conversation

@blooop

@blooop blooop commented Apr 20, 2026

Copy link
Copy Markdown
Owner

Designs a non-backward-compatible re-enablement of PR #688: nested
sub-configs (CacheCfg, ExecutionCfg, DisplayCfg, VisualizationCfg,
TimeCfg, RegressionCfg, ServerCfg) composed into BenchRunCfg via
param.ClassSelector slots, with no delegation magic and no flat-access
shims. Includes parameter-to-group mapping, migration steps, and
acceptance criteria.

Summary by Sourcery

Documentation:

  • Add BENCH_CFG_SPLIT_PLAN.md describing the target bench configuration module layout, parameter grouping, migration steps, and acceptance criteria for the future refactor.

claude added 2 commits April 20, 2026 18:25
Designs a non-backward-compatible re-enablement of PR #688: nested
sub-configs (CacheCfg, ExecutionCfg, DisplayCfg, VisualizationCfg,
TimeCfg, RegressionCfg, ServerCfg) composed into BenchRunCfg via
param.ClassSelector slots, with no delegation magic and no flat-access
shims. Includes parameter-to-group mapping, migration steps, and
acceptance criteria.
Treat the split as a clean-slate rewrite: cache hash was broken, so reset
it by bumping CACHE_VERSION rather than trying to preserve on-disk cache
compatibility. Frees hash_persistent to be simplified without constraint.
@sourcery-ai

sourcery-ai Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Adds a detailed design document (BENCH_CFG_SPLIT_PLAN.md) that specifies how to re-split bench_cfg.py into multiple param-based sub-configuration classes with a clean, non-backward-compatible API, covering target module layout, parameter grouping/renames, BenchRunCfg composition, migration steps, cache versioning, and acceptance criteria.

Sequence diagram for BenchRunCfg.from_cmd_line with sub-config CLI registration

sequenceDiagram
    actor User
    participant CLI as ArgParser
    participant BenchRunCfg
    participant ServerCfg
    participant ExecutionCfg
    participant CacheCfg
    participant DisplayCfg
    participant VisualizationCfg
    participant TimeCfg
    participant RegressionCfg

    User->>CLI: invoke with command line args
    BenchRunCfg->>ServerCfg: add_cli_args(parser)
    BenchRunCfg->>ExecutionCfg: add_cli_args(parser)
    BenchRunCfg->>CacheCfg: add_cli_args(parser)
    BenchRunCfg->>DisplayCfg: add_cli_args(parser)
    BenchRunCfg->>VisualizationCfg: add_cli_args(parser)
    BenchRunCfg->>TimeCfg: add_cli_args(parser)
    BenchRunCfg->>RegressionCfg: add_cli_args(parser)

    CLI-->>User: expose grouped flags
    User->>CLI: provides concrete flag values
    CLI-->>BenchRunCfg: parsed_args

    BenchRunCfg->>ServerCfg: construct from parsed_args
    BenchRunCfg->>ExecutionCfg: construct from parsed_args
    BenchRunCfg->>CacheCfg: construct from parsed_args
    BenchRunCfg->>DisplayCfg: construct from parsed_args
    BenchRunCfg->>VisualizationCfg: construct from parsed_args
    BenchRunCfg->>TimeCfg: construct from parsed_args
    BenchRunCfg->>RegressionCfg: construct from parsed_args

    BenchRunCfg-->>User: BenchRunCfg instance with nested sub-configs
Loading

Updated class diagram for the re-split bench configuration API

classDiagram
    class BenchRunCfg {
        +ServerCfg server
        +ExecutionCfg execution
        +CacheCfg cache
        +DisplayCfg display
        +VisualizationCfg visualization
        +TimeCfg time
        +RegressionCfg regression
        +String run_tag
        +Date run_date
        +__init__(**kwargs)
        +from_cmd_line(args)
        +with_defaults(run_cfg, execution_defaults, cache_defaults, display_defaults, visualization_defaults, time_defaults, regression_defaults, server_defaults)
        +deep()
    }

    class BenchCfg {
        +String bench_name
        +String description
        +String latex_name
        +String hash
        +String run_tag
        +Date run_date
        +raise_duplicate_exception()
        +hash_persistent()
        +describe_benchmark()
        +to_latex()
        +to_dict()
    }

    class DimsCfg {
        +List input_dims
        +List result_dims
        +List const_dims
    }

    class ServerCfg {
        +Integer port
        +List allow_ws_origin
        +Boolean show
        +add_cli_args(parser)
    }

    class ExecutionCfg {
        +Integer repeats
        +Integer level
        +Integer samples_per_var
        +String executor
        +Boolean nightly
        +Boolean headless
        +Boolean dry_run
        +Boolean only_plot
        +level_to_samples()
        +add_cli_args(parser)
    }

    class CacheCfg {
        +Boolean results
        +Boolean samples
        +Boolean clear
        +Boolean clear_samples
        +Boolean overwrite_samples
        +Boolean only_hash_tag
        +Integer size_mb
        +add_cli_args(parser)
    }

    class DisplayCfg {
        +Boolean print_bench_inputs
        +Boolean print_bench_results
        +Boolean summarise_constant_inputs
        +Boolean print_pandas
        +Boolean print_xarray
        +Boolean serve_pandas
        +Boolean serve_pandas_flat
        +Boolean serve_xarray
        +add_cli_args(parser)
    }

    class VisualizationCfg {
        +Boolean auto_plot
        +Boolean use_holoview
        +Boolean use_optuna
        +Integer plot_size
        +Integer plot_width
        +Integer plot_height
        +String pane_layout
        +String backend
        +add_cli_args(parser)
    }

    class TimeCfg {
        +Boolean over_time
        +Boolean clear_history
        +Integer max_events
        +Integer max_slider_points
        +Boolean show_aggregated_tab
        +Boolean show_aggregate_plots
        +String event
        +add_cli_args(parser)
    }

    class RegressionCfg {
        +Boolean enabled
        +String method
        +Float mad
        +Float percentage
        +Boolean fail
        +add_cli_args(parser)
    }

    BenchRunCfg <|-- BenchCfg

    BenchRunCfg o-- ServerCfg
    BenchRunCfg o-- ExecutionCfg
    BenchRunCfg o-- CacheCfg
    BenchRunCfg o-- DisplayCfg
    BenchRunCfg o-- VisualizationCfg
    BenchRunCfg o-- TimeCfg
    BenchRunCfg o-- RegressionCfg

    BenchCfg ..> DimsCfg
Loading

Flow diagram for migration steps to the new bench configuration structure

flowchart TD
    A[Start migration
1 Create bench_cfg package
with sub-config files
and BenchRunCfg BenchCfg DimsCfg] --> B[2 Update bencher.__init__
re-export new sub-config
classes and BenchRunCfg BenchCfg]
    B --> C[3 Migrate bencher library code
update flat attribute
accesses to grouped access]
    C --> D[4 Migrate bencher example files
use nested sub-config
attributes]
    D --> E[5 Migrate tests and scripts
update all run_cfg flat
attribute references]
    E --> F[6 Update documentation
how_to_use_bencher and
any gallery text]
    F --> G[7 Delete old bencher.bench_cfg.py
remove flat config class]
    G --> H[8 Bump CACHE_VERSION
in cache_management to
invalidate old caches]
    H --> I[9 Add focused unit tests
for sub-config defaults
composition with_defaults
deep hash_persistent]
    I --> J[10 Run pixi run ci
iterate until green]
    J --> K[End migration
acceptance criteria met]
Loading

File-Level Changes

Change Details Files
Introduce a written plan for refactoring bench_cfg.py into structured sub-config classes with a breaking API change and associated migration steps.
  • Document the guiding principle of having a single canonical nested access path for all configuration parameters, eliminating flat attribute access and delegation magic.
  • Define the target bencher/bench_cfg/ package layout with separate files for CacheCfg, ExecutionCfg, DisplayCfg, VisualizationCfg, TimeCfg, RegressionCfg, ServerCfg, BenchRunCfg, BenchCfg, and DimsCfg.
  • Map existing flat BenchRunCfg parameters into grouped sub-configs, including parameter renames and regrouping (e.g., cache, execution, time, regression, visualization, display, server).
  • Specify BenchRunCfg composition using param.ClassSelector fields for each sub-config, including initialization patterns ensuring fresh sub-config instances per BenchRunCfg.
  • Outline changes needed for key methods like from_cmd_line, with_defaults, deep, hash_persistent, and describe_benchmark in the new nested configuration model.
  • Provide a step-by-step call-site migration and repo-wide update plan, including code, examples, tests, scripts, docs, cache version bump, and changelog entries.
  • Clarify non-goals (no backward-compat shims, no delegation or property forwarding, no dataclass migration) and define acceptance criteria for the refactor.
BENCH_CFG_SPLIT_PLAN.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot 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.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@github-actions

Copy link
Copy Markdown

Performance Report for 781a845

Metric Value
Total tests 1301
Total time 120.55s
Mean 0.0927s
Median 0.0020s
Top 10 slowest tests
Test Time (s)
test.test_bench_examples.TestBenchExamples::test_example_meta 21.431
test.test_over_time_save_perf::test_save_faster_without_aggregated_tab 5.167
test.test_hash_persistent.TestCrossProcessDeterminism::test_hash_stable_across_two_processes[ResultBool] 4.318
test.test_over_time_repeats.TestMaxSliderPoints::test_default_subsampling_caps_at_max 3.120
test.test_generated_examples::test_generated_example[regression/example_regression_tuning_drift.py] 3.070
test.test_generated_examples::test_generated_example[cartesian_animation/example_cartesian_animation.py] 3.036
test.test_generated_examples::test_generated_example[regression/example_regression_tuning_noise.py] 3.030
test.test_generated_examples::test_generated_example[result_types/result_image/example_result_image_to_video.py] 2.881
test.test_generated_examples::test_generated_example[regression/example_regression_tuning_step.py] 2.571
test.test_bencher.TestBencher::test_combinations_over_time 1.461

Full report

Updated by Performance Tracking workflow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants