Conversation
Add configurable synthetic_ratio to control the proportion of synthetic vs benchmark data in each epoch (CLM-inspired replay). When set, each type's allocation is split between synthetic and benchmark source groups with weighted sampling within each group. - SamplerConfig: add synthetic_sources and synthetic_ratio fields - MultitaskSampler: add _split_sample method, use in sample_epoch - train_multitask.py: wire --synthetic-ratio CLI arg and config parsing with auto-detection of synthetic sources by name/path - All 5 multitask YAML configs: add commented-out synthetic_ratio option - 5 new tests covering ratio control, edge cases, and fallback behavior
There was a problem hiding this comment.
LGTM ✅ — Clean synthetic replay ratio support.
| Component | Status |
|---|---|
| Split sampling | ✅ _split_sample divides budget per type |
| Fallback | ✅ Falls back to available group when one side empty |
| Auto-detect | ✅ Sources with "synthetic" in name/path auto-tagged |
| CLI | ✅ --synthetic-ratio + YAML config |
| Tests | ✅ 5 tests (ratio control, edge cases, fallback) |
CLM-inspired 40% replay approach. The temporary _source_weights swap in _split_sample is pragmatic — works correctly and avoids duplicating the weighted sampling logic.
CI green.
There was a problem hiding this comment.
Approved ✅
Synthetic data replay ratio support (CLM-inspired):
Design
synthetic_ratio: target proportion of synthetic data per type (0.0–1.0)synthetic_sources: explicit set of source names, or auto-detected- Per-type split: each type's budget divided between synthetic and benchmark
Implementation
_split_sample(): separates sources into syn/bench groups, draws proportionally- Graceful fallback: if type has no synthetic sources → all from benchmark (and vice versa)
- Auto-detection: sources with "synthetic" in name/path tagged automatically
Wiring
- CLI:
--synthetic-ratio 0.4 - YAML:
sampling.synthetic_ratio,sampling.synthetic_sources
Tests (5 new)
test_synthetic_ratio_controls_proportion: ~40% at ratio=0.4test_synthetic_ratio_none_no_effect: disabled by defaulttest_synthetic_ratio_zero_excludes_synthetic: 0 synthetic itemstest_synthetic_ratio_one_all_synthetic: 100% synthetictest_synthetic_ratio_with_missing_synthetic_type: fallback per-type
CI green.
There was a problem hiding this comment.
Summary
Clean implementation of synthetic data replay ratio for CLM-style data mixing. The PR adds a configurable synthetic_ratio parameter to control the proportion of synthetic vs. benchmark data per question type during sampling.
Reviewed Items
| Component | Status | Notes |
|---|---|---|
SamplerConfig fields |
✅ | synthetic_sources: set[str], `synthetic_ratio: float |
_split_sample method |
✅ | Splits by source category, reuses _weighted_sample |
sample_epoch integration |
✅ | Routes to split/weighted based on config |
build_sampler_config |
✅ | Parses config + auto-detects synthetic sources |
CLI --synthetic-ratio |
✅ | Float arg, overrides config |
| Config YAMLs | ✅ | Commented examples in all 5 configs |
| Tests | ✅ | 5 new tests covering core logic + edge cases |
Implementation Highlights
- Backward compatible:
synthetic_ratio=None(default) preserves existing behavior - Graceful fallbacks: If a type has no synthetic (or benchmark) sources, draws all from the available side
- Integer rounding:
n_synthetic = int(n * ratio),n_benchmark = n - n_syntheticensures exact total - Auto-detection: Sources with "synthetic" in name or path are auto-classified
Minor Observation
The _split_sample temporarily mutates self._source_weights during sampling, then restores it. This works correctly because sample_epoch is single-threaded, but a comment noting this invariant would be helpful for future readers.
Not blocking — the pattern is safe as-is.
LGTM ✅
Summary
synthetic_ratioandsynthetic_sourcesparameters toSamplerConfigsynthetic_sourcesnot explicitly set--synthetic-ratio 0.4)Closes part of #97 (synthetic replay ratio approach)
Test plan
python -m pytest tests/test_sampler.py -v— 14 tests pass (9 existing + 5 new)