Modeled Coverage Phase 3 #620
Conversation
Improve logs for conflicting hexes, add log for seniority insert Remove logs, update uuid as well
Improve logs for conflicting hexes, add log for seniority insert Remove logs, update uuid as well
There was a problem hiding this comment.
Am I understanding correctly that the split between indoor and outdoor covered hexes is intended to be generic across both CBRS and Wifi hotspots, since CBRS gateways can have multiple radios, both indoor and outdoor (hypothetically) and thus might have results in both hashmaps while a wifi hotspot should only have a single set of either indoor or outdoor hashmap entries in any nearby covered hexes since they map 1-to-1 hotspot key/gateway-to-radio?
| epoch: &Range<DateTime<Utc>>, | ||
| _max_distance: f64, | ||
| max_distance: f64, |
There was a problem hiding this comment.
noting here reminder to derive wifi indoor from the coverage object rather than defaulting to indoor in the validate_heartbeat function
There was a problem hiding this comment.
I'm planning on doing that as a PR after.
There was a problem hiding this comment.
Brian mentioned yesterday that wifi outdoor should just work with this PR, but this wont be the case here where we default to wifi indoor. Wifi outdoor is also not supported in the cell_type.rs. As such it is not the case that wifi outdoor should just work. We should confirm expectation re wifi outdoor and this PR
There was a problem hiding this comment.
I recognize it went in already from a previous PR but I would really like KeyType and OwnedKeyType to get renamed to something that makes them distinct from the public key "key type" that PublicKeyBinary might actually refer to (RSA, Ed25519, EccCompact, etc). Something more like "Address" or "Identifier" which I think more clearly illustrates that the type is used to find a printable (or easily converted to printable) identifier
| @@ -253,7 +298,7 @@ impl HeartbeatReward { | |||
| } | |||
|
|
|||
| pub fn reward_weight(&self) -> Decimal { | |||
There was a problem hiding this comment.
this looks to be unsued now, any reason not to remove it?
| epoch: &Range<DateTime<Utc>>, | ||
| _max_distance: f64, | ||
| max_distance: f64, |
There was a problem hiding this comment.
Brian mentioned yesterday that wifi outdoor should just work with this PR, but this wont be the case here where we default to wifi indoor. Wifi outdoor is also not supported in the cell_type.rs. As such it is not the case that wifi outdoor should just work. We should confirm expectation re wifi outdoor and this PR
|
|
||
| let diff = expected_total_rewards - distributed_total_rewards; | ||
| let diff = expected_total_rewards as i128 - distributed_total_rewards as i128; |
There was a problem hiding this comment.
Why do we need i128 here? Nothing here should be breaching i64 limits and if they are its a bit of a red flag
There was a problem hiding this comment.
the expected could be greater or less, and these are u64s not i64s
| // confirm owner 1 reward is 0.1 of owner 2's reward | ||
| // owner 1 is a wifi indoor with a distance_to_asserted > max | ||
| // and so gets the reduced reward scale of 0.1 ( radio reward scale of 0.4 * location scale of 0.25) | ||
| // owner 2 is a cbrs sercomm indoor which has a reward scale of 1.0 |
There was a problem hiding this comment.
| // confirm owner 1 reward is 0.1 of owner 2's reward | |
| // owner 1 is a wifi indoor with a distance_to_asserted > max | |
| // and so gets the reduced reward scale of 0.1 ( radio reward scale of 0.4 * location scale of 0.25) | |
| // owner 2 is a cbrs sercomm indoor which has a reward scale of 1.0 | |
| // confirm owner 1 reward is 0.25 of owner 2's reward | |
| // owner 1 is a wifi indoor with a distance_to_asserted > max | |
| // and so gets a location scale of 0.25 | |
| // owner 2 is a cbrs sercomm indoor which has a location scale of 1.0 |
No description provided.