Set activity fix for miners in metagraph #245
Conversation
There was a problem hiding this comment.
Thanks for contributing and please see my comments.
I don't believe this solution is ideal as it would be expensive in your current revision.
Perhaps a better solution is to remove the notation in the client code (i.e. btcli and TAOStats) as well as the docs.
| @@ -181,15 +181,22 @@ impl<T: Config> Pallet<T> { | |||
| // --- 17. Set the activity for the weights on this network. | |||
| Self::set_last_update_for_uid(netuid, neuron_uid, current_block); | |||
|
|
|||
| // --- 18. Emit the tracking event. | |||
| // --- 18. Set the activity for the minors on this network. | |||
| for &uid in uids.iter() { | |||
There was a problem hiding this comment.
There is no logic to check if the uid is a validator or a miner
| @@ -181,15 +181,22 @@ impl<T: Config> Pallet<T> { | |||
| // --- 17. Set the activity for the weights on this network. | |||
| Self::set_last_update_for_uid(netuid, neuron_uid, current_block); | |||
|
|
|||
| // --- 18. Emit the tracking event. | |||
| // --- 18. Set the activity for the minors on this network. | |||
There was a problem hiding this comment.
Why should this run every time any key sets weights? This will use up a lot of resources each call to set_weights.
There was a problem hiding this comment.
Thanks for you feedback! As far as I know, in the older version, every miner was supposed to call this set_weight function alongside the validators every weights_rate_limit blocks, based on the subtensor parameter. Assuming there were 256 neurons in the subnet, that would have been 256 times every weights_rate_limit blocks. In the new update, since only validators can set weights, my assumption was that they have to loop over all the UIDs to which they are setting weights and update their 'UDATED' value in the metagraph as well. I do agree that this does not bring efficiently if the idea behind miners not setting weights was to gain efficiency.
There was a problem hiding this comment.
I do agree that a better solution could be to ignore the last updated block value for the miners from the metagraph (not sure if it is important to know when a weight has been written for the miner). Assuming that the latest miner code template needs to be adjusted to avoid entering an infinite loop seeking metagraph resync when the last updated block is older than some threshold, which would otherwise leave the miner stuck in the loop resyncing. I'm working on a new subnet code, and that is why I have ended up in this PR. :)
There was a problem hiding this comment.
not sure if it is important to know when a weight has been written for the miner
This part is the key issue. It has been quite a while since the change that miners do not set weights, so I hesitate to see any re-adoption of a similar mechanism to be useful. If anything, we should remove mentions to this in the client code.
Since miners don't set weights anymore, the value of the last 'UPDATE' value in the metagraph remains untouched for miners. The current 'set_weights' function correctly updates the weights for the uids but only updates the activity value in the graph for the validator itself. As a result, the last block updated for those miners in the list of uids for set_weights remains untouched and keeps growing. This raises 2 problem: