-
Notifications
You must be signed in to change notification settings - Fork 1
fix: structural issue batch — bounded reads, oracle coherence, protocol comparison, vLLM percentiles #293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: structural issue batch — bounded reads, oracle coherence, protocol comparison, vLLM percentiles #293
Changes from 13 commits
4e243b4
4252a68
dd9148c
f2133ac
02646ca
0486592
17e945a
b4d6734
39cb8cc
529aefb
7d3ef1b
9cf4a3f
cd57b18
0a221bf
7aeb3c6
d79dd8e
e6ba710
6c19bc1
6ebc992
4762de6
2ee72e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,10 @@ | |
| _FactorExperimentConfig, | ||
| _OutcomeExperimentConfig, | ||
| _ScaledLegacyExperimentConfig, | ||
| scalar_contains, | ||
| scalar_equal, | ||
| scalar_identity_set, | ||
| scalar_subset, | ||
| ) | ||
| from flameox.catalog import Catalog | ||
| from flameox.domain import ( | ||
|
|
@@ -139,6 +143,7 @@ class ExperimentPlan(ContractModel): | |
| execution_policy: ExecutionPolicy | ||
| variant_parameter: str | ||
| variants: tuple[str, ...] | ||
| baseline_variant: str | None = None | ||
| factors: dict[str, tuple[JsonValue, ...]] = Field(default_factory=dict) | ||
| parameter_overrides: dict[str, JsonValue] | ||
| blocks: tuple[ExperimentBlock, ...] | ||
|
|
@@ -660,6 +665,11 @@ async def plan( | |
| execution_policy=execution_policy, | ||
| variant_parameter=variant_parameter, | ||
| variants=variants, | ||
| baseline_variant=( | ||
| self._factor_label(config.baseline_value) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a treatment factor contains type-distinct values with the same display label, such as integer AGENTS.md reference: AGENTS.md:L46-L49 Useful? React with 👍 / 👎. |
||
| if isinstance(config, _FactorExperimentConfig) and config.baseline_value is not None | ||
| else None | ||
| ), | ||
| factors={ | ||
| name: tuple(cast(JsonValue, value) for value in values) | ||
| for name, values in factors.items() | ||
|
|
@@ -903,12 +913,26 @@ async def report(message: str) -> None: | |
| "Automatic experiment comparison currently requires pyperf measurements." | ||
| ) | ||
| else: | ||
| comparison_run_sets: tuple[RunSet, ...] = run_sets | ||
| if not plan.baseline_variant: | ||
|
morluto marked this conversation as resolved.
Outdated
|
||
| limitations.append( | ||
| "Baseline was determined by list position, not an explicit " | ||
| "baseline_value. Reordering the treatment list reverses the " | ||
| "comparison direction." | ||
| ) | ||
| else: | ||
| comparison_run_sets = tuple( | ||
| sorted( | ||
| run_sets, | ||
| key=lambda run_set: run_set.selection["variant"] != plan.baseline_variant, | ||
|
morluto marked this conversation as resolved.
Outdated
|
||
| ) | ||
| ) | ||
| comparison = await run_atomic_thread( | ||
| lambda: ComparisonService(self.workspace).record( | ||
| parse_compare_run_sets_request( | ||
| { | ||
| "baseline_run_set_id": run_sets[0].run_set_id, | ||
| "candidate_run_set_id": run_sets[1].run_set_id, | ||
| "baseline_run_set_id": comparison_run_sets[0].run_set_id, | ||
| "candidate_run_set_id": comparison_run_sets[1].run_set_id, | ||
| "experiment_id": plan.experiment.experiment_id, | ||
| "metric": plan.experiment.primary_metric, | ||
| "unit": ( | ||
|
|
@@ -988,7 +1012,7 @@ def _materialize_combinations( | |
| matches = [ | ||
| name | ||
| for name, choices in workload_parameters.items() | ||
| if set(config.variants).issubset(set(choices)) | ||
| if scalar_subset(list(config.variants), list(choices)) | ||
| ] | ||
| if not matches: | ||
| raise DomainError( | ||
|
|
@@ -1018,7 +1042,9 @@ def _materialize_combinations( | |
| ErrorCode.WORKSPACE_INVALID, | ||
| f"Experiment factor {name!r} is not a workload parameter.", | ||
| ) | ||
| if len(set(values)) != len(values) or not set(values).issubset(set(allowed)): | ||
| if len(scalar_identity_set(list(values))) != len(values) or not scalar_subset( | ||
| list(values), list(allowed) | ||
| ): | ||
|
morluto marked this conversation as resolved.
|
||
| raise DomainError( | ||
| ErrorCode.WORKSPACE_INVALID, | ||
| f"Experiment factor {name!r} contains duplicate or undeclared values.", | ||
|
|
@@ -1040,7 +1066,7 @@ def _materialize_combinations( | |
| ErrorCode.WORKSPACE_INVALID, | ||
| "Explicit combinations must contain every declared factor exactly once.", | ||
| ) | ||
| if any(combination[name] not in factors[name] for name in factor_names): | ||
| if any(not scalar_contains(combination[name], factors[name]) for name in factor_names): | ||
| raise DomainError( | ||
| ErrorCode.WORKSPACE_INVALID, | ||
| "Explicit combination contains an undeclared factor value.", | ||
|
|
@@ -1060,7 +1086,7 @@ def _materialize_combinations( | |
| ErrorCode.WORKSPACE_INVALID, | ||
| "Every exclusion must name at least one declared factor.", | ||
| ) | ||
| if any(value not in factors[name] for name, value in rule.items()): | ||
| if any(not scalar_contains(value, factors[name]) for name, value in rule.items()): | ||
| raise DomainError( | ||
| ErrorCode.WORKSPACE_INVALID, | ||
| "Exclusion contains an undeclared factor value.", | ||
|
|
@@ -1069,7 +1095,7 @@ def _materialize_combinations( | |
| combination | ||
| for combination in combinations | ||
| if not any( | ||
| all(combination[name] == value for name, value in rule.items()) | ||
| all(scalar_equal(combination[name], value) for name, value in rule.items()) | ||
| for rule in config.exclude | ||
| ) | ||
| ) | ||
|
|
@@ -1102,7 +1128,10 @@ def _outcome_result( | |
| selected = [ | ||
| trial | ||
| for trial in trials | ||
| if trial.factors.get(plan.variant_parameter) == treatment_value | ||
| if scalar_equal( | ||
| cast(Scalar, trial.factors.get(plan.variant_parameter)), | ||
| cast(Scalar, treatment_value), | ||
| ) | ||
| ] | ||
| attempted = sum(trial.outcome is not TrialOutcome.UNATTEMPTED for trial in selected) | ||
| eligible = sum( | ||
|
|
@@ -1211,9 +1240,13 @@ def _outcome_result( | |
| disposition = ExperimentOutcomeDisposition.INSUFFICIENT_EVIDENCE | ||
| elif not failures: | ||
| disposition = ExperimentOutcomeDisposition.ALL_CLEAN | ||
| elif len(plan.variants) == 2 and failed_treatments == {plan.variants[0]}: | ||
| elif len(plan.variants) == 2 and failed_treatments == { | ||
| plan.baseline_variant or plan.variants[0] | ||
| }: | ||
| disposition = ExperimentOutcomeDisposition.BASE_ONLY_FAILURE | ||
| elif len(plan.variants) == 2 and failed_treatments == {plan.variants[1]}: | ||
| elif len(plan.variants) == 2 and failed_treatments == { | ||
| v for v in plan.variants if v != (plan.baseline_variant or plan.variants[0]) | ||
| }: | ||
| disposition = ExperimentOutcomeDisposition.CANDIDATE_ONLY_FAILURE | ||
| else: | ||
| disposition = ExperimentOutcomeDisposition.MIXED | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.