(7/n) Complete declarative task runtime and shared pairwise orchestration - #86
(7/n) Complete declarative task runtime and shared pairwise orchestration#86kargibora wants to merge 4 commits into
Conversation
cbc94ff to
06a7211
Compare
| @@ -0,0 +1,79 @@ | |||
| """Public schema API for declarative task definitions. | |||
There was a problem hiding this comment.
Note:
All these related changes simply splits functions. As mt_bench can require some specific fields that others ignore, we only need a shared components + special components on their own file.
| @@ -0,0 +1,43 @@ | |||
| """Baseline-selection policies shared by pairwise protocols.""" | |||
There was a problem hiding this comment.
This was a design problem we have talked before. Baseline is used to determine which model should be used as default - and which tasks requires a baseline provided by the config file we send. We probalby dont need many fields other than the ones that requires us to introduce functionality like using multiple different baselines sucnh asCategoryDefaultBaseline. However if you think this is over-engineering happy to remove it
There was a problem hiding this comment.
It is not clear why we need it.
Why cant we just have a baseline: str = "gpt-4o" config key with a default value in the task configurations?
Move dataset, baseline, pairwise, and MT-Bench contracts into focused modules while preserving the public schema imports and YAML behavior.
Explain task identity, adding YAML definitions, and when new dataset adapters or protocol schemas are required.
Pass resolved tasks through dispatch, normalize registered dataset inputs behind one contract, and move baseline planning out of the runner.
Let prompt presets own parsing, resolve scoring adapters at runtime, and remove repeated metric semantics from task YAML.
06a7211 to
d16cdfb
Compare
|
|
||
|
|
||
| def run_mt_bench_benchmark(cfg: RunConfig): | ||
| def run_mt_bench_benchmark( |
There was a problem hiding this comment.
This feels slightly weird to me. I would imagine that tasks resolve a run function which gets executed.
Here it feels weird to have run_mt_bench_benchmark takes as input cfg: RunConfig and _resolved_task: ResolvedTaskSpec.
| return sorted(self.baseline_by_index.dropna().unique().tolist()) | ||
|
|
||
| @property | ||
| def is_flat(self) -> bool: |
There was a problem hiding this comment.
why is_flat rather than is_unique_model?
| @@ -0,0 +1,43 @@ | |||
| """Baseline-selection policies shared by pairwise protocols.""" | |||
There was a problem hiding this comment.
It is not clear why we need it.
Why cant we just have a baseline: str = "gpt-4o" config key with a default value in the task configurations?
|
|
||
| import pandas as pd | ||
|
|
||
| from judgearena.utils.eval import PrefSummary, compute_pref_summary |
There was a problem hiding this comment.
are we using compute_pref_summary anywhere else? if not lets put the code in the object bellow to simplify.
|
|
||
| _PAIRWISE_SCORERS = { | ||
| "pairwise_win_rate": PairwiseScorer( | ||
| name="pairwise_win_rate", |
There was a problem hiding this comment.
DRY, name appears twice.
Either we keep the dictionary and rename name field (best approach I think) or we have a list of PairwiseScorer.
| required = {"instruction_index", "model", "output"} | ||
| missing = sorted(required - set(self.model_outputs.columns)) | ||
| if missing: | ||
| raise ValueError( |
There was a problem hiding this comment.
This should not happen when querying the object but when creating it.
| if preloaded is not None: | ||
| return preloaded.loc[instructions.index] | ||
| else: | ||
| preloaded = _try_load_legacy_dataset_completions( |
There was a problem hiding this comment.
cant we put the logic above in _try_load_legacy_dataset_completions?
(I mean this
if task_data is not None:
preloaded = task_data.completions_for(model_spec)
if preloaded is not None:
return preloaded.loc[instructions.index]
else:
)
otherwise the code is weird since the block is also trying to load dataset completions.
| from judgearena.tasks.schema.base import StrictFrozenModel | ||
|
|
||
|
|
||
| class HuggingFaceDatasetSource(StrictFrozenModel): |
There was a problem hiding this comment.
Do we really need those sources?
Dataset are always loaded locally, we have code to download them so things are reproducible given a git hash.
Can we remove this as a simplification given that the PR is quite complex?
| @@ -0,0 +1,113 @@ | |||
| # Task definitions | |||
There was a problem hiding this comment.
I wish I would have started the PR by this 😂.
| ## Layout | ||
|
|
||
| ```text | ||
| tasks/ |
There was a problem hiding this comment.
We do have one folder per task right? (also we should support custom files from the folder so for instance mt_bench.py should probably be in mt_bench/)
Summary
This PR connects packaged task YAML definitions to dataset loading, baseline selection, benchmark execution, and scoring, and splits the task schema into focused modules.
Previously, task definitions described benchmark settings, but parts of the runtime still selected behavior through hard-coded branches, repeated registry lookups, or direct function calls. Adding another task could therefore require changes across several unrelated files.
After this change, a task is resolved once and passed through the pairwise evaluation pipeline. Existing components can be reused by referencing their registered names from task YAML.
Before and after
Architecture
The runtime now follows this path:
The new schema package separates datasets, sources, baselines, pairwise protocols, MT-Bench protocols, and resolved task models. This prevents a single schema file from growing whenever a new benchmark family introduces specialized fields.
Most new tasks do not require a new schema or runner. A task using existing components only needs a YAML definition selecting its dataset adapter, runner, baseline policy, judge protocol, and scorer.
A new Python component is only necessary when a benchmark introduces genuinely different behavior:
Executable behavior remains in Python rather than YAML; YAML only selects registered and validated components.
Changes
primary_metric/higher_is_better/parserwere removed from task YAML since the scorer and prompt preset already own them.Notes
Most of the changes are coming from splitting
schema.pyto multiple files, thus this is just a refactoring step as next PR's will introduce two more task which can inflate the same file if they require some special fields.