Clamp negative orig_m #2308
Clamp negative orig_m #2308
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements handling for negative orig_m values in training data by clamping them to 0.0f instead of failing validation. The change removes a strict validation assertion and adds a normalization function that warns about and corrects negative values.
- Removes strict validation that rejected negative
orig_mvalues - Adds
NormalizeOriginalMetricsfunction to clamp negativeorig_mvalues to 0.0f with warnings - Integrates the normalization step into the file processing pipeline
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/trainingdata/rescorer.cc | Removes validation assertion and adds normalization function for orig_m values |
| libs/lczero-common | Updates subproject commit reference |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/trainingdata/rescorer.cc
Outdated
| std::cerr << "Warning: negative orig_m (" << chunk.orig_m | ||
| << ") encountered; clamping to 0" << std::endl; |
There was a problem hiding this comment.
Using std::cerr for warnings may not be appropriate for production code. Consider using a proper logging framework or configuration-controlled warning system instead of direct stderr output.
|
I think this is the wrong approach, we should just drop weird data. If we want to fix something it should be on the generating side. |
|
The backend bug (mish instead of relu as final activation) is fixed, but is the |
|
its not critical at the current time, but we have copious amounts of data which passes all these checks, so it seems unnecessary to try and allow this small amount of extra data through at rescorer time - and increase the chance that we miss such a bug again in future. |
3 participants
No description provided.