Skip to content

Conversation

@casteryh
Copy link
Contributor

Fix bug in age_evict() where max_age=0 and max_samples=0 are incorrectly treated as falsy
values. This is buggy for pure on-policy training (max_age=0). Changed to use explicit None checks instead of
truthiness checks.

The age_evict function was using truthiness checks (if max_age and ...)
which incorrectly evaluates to False when max_age=0 or max_samples=0.
This prevents the eviction policy from working correctly when these
parameters are set to 0 (which are valid values).

Changed to use explicit None checks (if max_age is not None and ...)
to properly handle 0 as a valid parameter value.

Test Plan:
- Verify that age_evict correctly evicts entries when max_age=0
- Verify that age_evict correctly evicts entries when max_samples=0
- Existing tests should continue to pass
@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 12, 2025
@casteryh casteryh marked this pull request as ready for review November 12, 2025 01:01
@casteryh casteryh requested a review from joecummings November 12, 2025 01:10
@felipemello1 felipemello1 merged commit 448c18a into main Nov 12, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants