google-deepmind / google-deepmind/reverb

Unnecessary assert in prioritized selector

Open
#135 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
793
Forks
113
PR merge metrics
No merged PRs in 30d

Description

@samlobel and I are trying to create a replay table with prioritized sampler _and_ remover. We are trying to remove low priority entries by negating the `priority_exponent` argument to the remover's constructor (the `priority_exponent` converts the priority scores to probabilities by raising the score to the exponent and dividing by the sum of scores raised to exponent). However, there is an assert in the cpp constructor for the remover selector that prevents us from specifying negative `priority_exponent`s, even though this is a totally valid mathematical operation which would continue to yield valid sampling/removing probabilities. Here is an example culprit assert of code: https://github.com/google-deepmind/reverb/blob/7e9ee1ab4aa042a50b50c87820708a8a91b73906/reverb/cc/selectors/prioritized.cc#L60.

Would it be possible to remove that assert statement? Alternatively, is there a workaround?

Contributor guide

Open the contributing guide

Research direction

Start in reverb/cc/selectors/prioritized.cc at the remover selector constructor assertion linked in the issue. Check the existing selector behavior and tests, then verify that negative priority_exponent values are accepted and still produce valid sampling or removing probabilities.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
machine-learning
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.