Add difficulty-proportional type ratio override - #101
Conversation
There was a problem hiding this comment.
LGTM ✅ — Clean difficulty-proportional sampling.
| Component | Status |
|---|---|
| Override | ✅ difficulty_weights overrides type_ratios when set |
| Fallback | ✅ effective_ratios = difficulty_weights or type_ratios |
| CLI | ✅ --difficulty-weights '{...}' JSON string |
| Tests | ✅ 3 tests (override, fallback, missing type) |
Addresses data quantity (vs per-type LR which only adjusted gradient scale). CI green.
There was a problem hiding this comment.
Approved ✅
Difficulty-proportional type ratio override:
Design
difficulty_weightsoverridestype_ratiosfor sampling- Controls data quantity per type, not learning rate
- Inverse-accuracy weighting: harder types get more training data
Example
difficulty_weights:
noul: 0.5 # ~80% accuracy → low weight
choice: 1.0 # ~61% accuracy → medium weight
score: 2.0 # ~50% accuracy → high weight (4x noul)Implementation
effective_ratios = self._config.difficulty_weights or self._config.type_ratios- Clean override pattern — no special casing scattered through code
- Used in both
items_per_typeallocation and window slot allocation
CLI
--difficulty-weights '{"noul": 0.5, "choice": 1.0, "score": 2.0}'- Parsed as JSON string
Tests (3 new)
- Override behavior (score gets ~4x noul)
- None falls back to type_ratios
- Missing type handled gracefully
CI green.
There was a problem hiding this comment.
LGTM! Clean implementation of difficulty-proportional sampling.
Review Summary
| Aspect | Status |
|---|---|
| CI | ✅ All 4 checks passed |
| Logic | ✅ Correct override mechanism |
| Tests | ✅ 3 new tests cover key scenarios |
| Config | ✅ YAML + CLI wiring complete |
Implementation Notes
-
Override pattern is sound:
effective_ratios = difficulty_weights or type_ratios— simple, explicit, no partial merge. -
Consistent application:
effective_ratiosused in all 4 locations:- Initial type allocation (L174, L182)
- Window slot allocation (L220, L227)
-
Missing-type handling: Test confirms graceful behavior when
difficulty_weightsreferences absent types (score in noul+choice pool). -
CLI integration:
--difficulty-weights '{"noul": 0.5, "choice": 1.0, "score": 2.0}'parses correctly viajson.loads. -
Config docs: Commented examples in all 5 YAML configs provide usage guidance without changing defaults.
Minor observation (not blocking): The docstring says "overrides type_ratios for sampling" — this is accurate for proportions but doesn't affect the window-interleaving structure itself, which is expected.
Add difficulty_weights field to SamplerConfig that overrides type_ratios when set, allowing harder question types (lower accuracy) to receive more training data. Wire through train_multitask.py with both YAML config and --difficulty-weights CLI support. Add commented-out examples to all 5 multitask config files.
a527725 to
5264509
Compare
Summary
difficulty_weightsparameter toSamplerConfigas a semantic override fortype_ratios{noul: 0.5, choice: 1.0, score: 2.0}— score (~50% acc) gets 4x the data of noul (~80% acc)--difficulty-weights '{"noul": 0.5, "choice": 1.0, "score": 2.0}')Closes part of #97 (difficulty-proportional sampling approach)
Test plan
python -m pytest tests/test_sampler.py -v— 12 tests pass (9 existing + 3 new)