Random Activity Generator #113
Conversation
carlaKC
force-pushed
the
111-randomactivity
branch
3 times, most recently
from
144f971 to
f363587
Compare
carlaKC
force-pushed
the
111-randomactivity
branch
from
f363587 to
0c1d131
Compare
carlaKC
force-pushed
the
111-randomactivity
branch
from
0c1d131 to
0b5ca09
Compare
|
Thanks for the review @sr-gi! I've just resolved some of the obvious rust improvements, but please re-open if I missed any of them (and thank you for 🦀 wisdom). My main takeaway is that we need to think about the ways that our magical I wonder whether we shouldn't just prune anything where: Because that would mean that we always succeed in picking a payment amount, which would simplify things a little. We can log warnings when we do this pruning so that people know to re-configure (or leave as-is) their simulation? Main changes on push are: rust cleanups, lots of docs and move retries into |
carlaKC
force-pushed
the
111-randomactivity
branch
from
0b5ca09 to
405ae4a
Compare
carlaKC
force-pushed
the
111-randomactivity
branch
2 times, most recently
from
2f1c4be to
45c4d95
Compare
carlaKC
force-pushed
the
111-randomactivity
branch
from
45c4d95 to
f2740e9
Compare
carlaKC
force-pushed
the
111-randomactivity
branch
from
f2740e9 to
8116679
Compare
|
Jumped the gun on rebasing on #104 so diff from last review will be a bit muddled (apologies!). Major change is just adding the loop for selecting a destination and logging if we can't find one after an unreasonably high number of retries + some validation to ensure that we have at least 2 nodes for |
carlaKC
force-pushed
the
111-randomactivity
branch
from
8116679 to
9fc4a06
Compare
When we add random activity generation, we're going to want consumers for every node that we have execution on. Previously, we only ran consumers for nodes that are listed as the source in activity description. This change separates consumer and producer spin up and allows the caller to specify which nodes to run consumers for.
carlaKC
force-pushed
the
111-randomactivity
branch
2 times, most recently
from
ddffe97 to
d896cdd
Compare
When we want to simulate random activity in large network, keeping track of our graph will be the most expensive part of the simulation. This is because we'll want to cache node capacities for easy lookup, and because a weighted index has a large memory footprint (N -1 u64s for our use). To minimize the cost of tracking these values, we use a single NetworkGraphView tracker that will provide expensive operations in a single location.
Allow parsing of empty activity descriptions to facilitate default run with random activity generation. We still use a vector in the Simulation struct to allow a path where we have a combination of random and specific activity. This commit also adds validation that we have at least two nodes when running random activity. This is required because destination nodes are chosen from within the nodes that we have execution on (to prevent liquidity draining and eventual death spirals).
carlaKC
force-pushed
the
111-randomactivity
branch
from
d896cdd to
46566b2
Compare
|
Merging this. A post-merge review would be nice if you find some time @okjodom |
Ack. I haven't been able to dive deep into this one but will do |
|
I'm happy dancing after looking through this :) |
Fixes #72 (see #111 for design doc).
Some design considerations:
WeightedIndex) is where we take the biggest memory hit, so that's pulled out into its own manager which all tasks use (ie, we don't want to make copies of it).