Add difficulty map builder#1693
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a difficulty-aware curriculum sampling framework for RLVR and GRPO training. It includes a new sampler capable of both static scheduling and adaptive sampling based on reward and advantage signals, alongside a utility script for generating difficulty metadata from Hugging Face datasets using Beta-Binomial posterior estimation. Comprehensive unit tests are provided for both components. Reviewer feedback focuses on enhancing code robustness and clarity by adding missing type hints, specifying UTF-8 encoding for file operations, and removing redundant conditional logic in serialization helpers.
|
|
||
|
|
||
| def _build_difficulty_bucket_index( | ||
| dataset, |
There was a problem hiding this comment.
The dataset parameter is missing a type hint. Adding one (e.g., Any or a more specific sequence type) improves code clarity and maintainability.
| dataset, | |
| dataset: Any, |
|
|
||
| def __init__( | ||
| self, | ||
| dataset, |
There was a problem hiding this comment.
The dataset parameter in the __init__ method is missing a type hint.
| dataset, | |
| dataset: Any, |
| *, output_jsonl: Path, schema_json: Path, metadata_json: Path, dataset: Dataset, dataset_metadata: DatasetMetadata | ||
| ) -> None: | ||
| output_jsonl.parent.mkdir(parents=True, exist_ok=True) | ||
| with output_jsonl.open("w") as output_file: |
There was a problem hiding this comment.
It is recommended to specify the encoding when opening files for writing to ensure consistency across different platforms, especially for JSON data which should be UTF-8.
| with output_jsonl.open("w") as output_file: | |
| with output_jsonl.open("w", encoding="utf-8") as output_file: |
| schema_payload: Any = dataset.features.to_dict() | ||
| except AttributeError: | ||
| schema_payload = str(dataset.features) | ||
| with schema_json.open("w") as output_file: |
There was a problem hiding this comment.
Specify encoding="utf-8" when opening the schema file for writing to ensure consistent behavior across environments.
| with schema_json.open("w") as output_file: | |
| with schema_json.open("w", encoding="utf-8") as output_file: |
| json.dump(schema_payload, output_file, indent=2, sort_keys=True) | ||
|
|
||
| metadata_json.parent.mkdir(parents=True, exist_ok=True) | ||
| with metadata_json.open("w") as output_file: |
There was a problem hiding this comment.
Specify encoding="utf-8" when opening the metadata file for writing.
| with metadata_json.open("w") as output_file: | |
| with metadata_json.open("w", encoding="utf-8") as output_file: |
| (key if isinstance(key, str) else "" if key is None else str(key)): make_jsonable(item) | ||
| for key, item in value.items() | ||
| } | ||
| return "" if value is None else str(value) |
There was a problem hiding this comment.
The check value is None is redundant here because None is already handled at line 1130. This line can be simplified to just return the string representation.
| return "" if value is None else str(value) | |
| return str(value) |
…' into split-difficulty-curriculum-core
…o split-create-difficulty-map
…iculum-core # Conflicts: # CHANGELOG.md
1 participant
No description provided.