Cosmos-sdk: simulation improvements / discussion

[rigelrozanski]

ideas of how to improve the simulation (living issue) code from a code clarity perspective (aka not feature dependant)

• split up core simulation functionality into multiple functions
  
  
  
  • within simulation (x/mock/simulation/simulation.go) there is a "master" simulation function SimulateFromSeed - this function is ~200LOC and should probably be broken up into 3-5 functions for better code comprehension, I imagine the contents of that giant loop should be broken out for example.
  
  
• Introduce more structs and have more core functionality as property on these structs
• Inputs to many of these functions are quite large. Where possible, and where there are common groups of function input, we should consider creating structures to group the common elements (and even better creating property functions on them)
• Craft interfaces around event statistics and logging, right now we just pass in function variables which are called, it would be good to turn these into interfaces, checkout the design approach for the event statistics logger which is a part of the first cleanup PR (#2720 - see x/mock/simulation/event_stats.go) - this could be further extended with an interface
• We should probably have a single output flow, right now it's a mix-n-match between the logger and print statements - I find this confusing

Side note, I know that currently there are many operations within the simulator which are heavily optimized, _however_ at the expense of code-clarity. We need to strike a balance here between performance and comprehension - I think that as this is a simulation, we should focus of code clarity for overall structure, and focus on adding new features to improve performance (such as the ability to save the state and start running the simulation from a certain block)

CC @cwgoes

[ValarDragon]

Side note, I know that currently there are many operations within the simulator which are heavily optimized, however at the expense of code-clarity. We need to strike a balance here between performance and comprehension - I think that as this is a simulation, we should focus of code clarity for overall structure, and focus on adding new features to improve performance (such as the ability to save the state and start running the simulation from a certain block)

What are you referring to as optimizations at the expense of code clarity?

[cwgoes]

Broadly, I agree with all the bulleted suggestions. I think we could also more clearly separate out the "Tendermint simulation" part from the "state machine fuzzer" part, clean up cmd/gaia/app/sim_test.go, which is a mess, and make the random genesis logic more generic so other SDK applications can reuse it.

I do not agree that code clarity is worth sacrificing performance - more performance means more state space covered by the simulation (and more of the real state machine being tested). I also don't think code clarity and performance are necessarily at odds - are there particular instances where you think we could only gain clarity at the expense of performance?

[alexanderbez]

There must be a way to have best of both worlds. We just need to strike a balance 💯

[rigelrozanski]

Maybe I am incorrect in thinking that some of the structure which I saw is for optimization. Some of the operations I understood from a brief previous discussed with @cwgoes had to do with taking multiple pieces of logic and grouping them together with function variables to avoid having to allocate more memory. aka stuff like:

func (a b c ) (d func ){
   ... do stuff ... with a b c 
   d := func{
       ...... do stuff ... with a b c  ...   
   }
   return d
}

In certain circumstances it's possible that maybe there is a code structure which is less nested/spagetti which is less efficient but we should probably just be using for clarity purposes. HOWEVER I do agree with @alexanderbez that there _probably_ is a way to get both efficiency and clarity - I just strongly feel that we should favour clarity over minor losses in performance, and then use that as a starting place to work towards greater performance. Here I would define a minor loss in performance as maybe doubling computational time (so maybe this is were opinions diverge @cwgoes). Again this is a simulation not a live network, so we can give ourselves a bit more leeway than would otherwise be acceptable

CC @ValarDragon

[jackzampolin]

@fedekunze just completed a simulation refactor. Going to close this issue as addressed. Please reopen with actionable items if applicable.

[fedekunze]

The only thing left is to split the core simulation into multiple pieces. Not high priority tho