generated from opentensor/bittensor-subnet-template
-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
samples uids only on candidate uids #142
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
steffencruz
reviewed
Mar 6, 2024
steffencruz
approved these changes
Mar 12, 2024
steffencruz
approved these changes
Mar 12, 2024
steffencruz
approved these changes
Mar 12, 2024
p-ferreira
approved these changes
Mar 12, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current implementation of uid sampling is very weird, and I don't understand it. Currently, if the number of uids is < the number of samples we want in a batch (k) (which is rare), we will take all the samples in the network, and then sample uids that are NOT in the candidate uids AND in the
avail_uids
variable. Therefore, this samples uids that are explicitly excluded from from the original set of candidate uids. Unless I am mis-reading this, it seems like a very strange implementation to me. The reasoning for this, for what I can tell, is to sample uids until we reachk
, even if the uids are in the exclusion category.I don't like this implementation, and it requires the validator to set a value of
k
that they won't have prior knowledge to beforehand. In production, this isn't an issue, since we will likely always havecandidate_uids
>>k
. However, in testnet scenarios, you have no idea. I ran into many issues regarding sampling when trying to deploy a validator on testnet because of the logic of sampling.Therefore, I propose a different uid sampling procedure, where we do not artificially want the batch to be 50, but rather the batch is always capped to be the max number of candidate uids if
candidate_uids
<k
. Otherwise, we randomly sample ONLY from the list ofcandidate_uids
. This removes the need for theavailable_uids
list.