Optimize forward step for large populations#10
Optimize forward step for large populations#10maxsvetlik wants to merge 11 commits intoSonyResearch:mainfrom
Conversation
|
@maxsvetlik Thanks for the PR - can you please fix the style checks? |
|
FWIW prior to implementing these changes, I had focused effort parallelizing the forward update step across multiple threads or processes, but was unsuccessful due to the overhead involved. I believe this is due to the nature of the computations happening (numerous short calls to objects). |
|
Absolutely. I'll push a commit to address the CI issues shortly. |
|
Apologies for gumming up the CI with failed builds. Didn't realize I could run it locally with |
|
Ah, sorry about finding about the |
| return a[idx // b.size], b[idx % b.size] | ||
|
|
||
|
|
||
| def comb2_reduced(l: np.array, idx: list) -> tuple: |
There was a problem hiding this comment.
Could you improve the documentation on the arguments and what is happening inside the function?
|
|
||
|
|
||
| @lru_cache(maxsize=None) | ||
| def nCk(n: int, k: int) -> int: |
There was a problem hiding this comment.
Same comment as above
| calulate binomial coeffecients | ||
| """ | ||
| m = 0 | ||
| if k == 0: |
There was a problem hiding this comment.
Could you comment on these conditions?
|
|
||
|
|
||
| @lru_cache(maxsize=None) | ||
| def nCk(n: int, k: int) -> int: |
There was a problem hiding this comment.
Can you improve the naming of this function?
|
@maxsvetlik any updates to @rcapobianco's comments? Those seem very minor and we can merge this PR once you address them. Thanks. |
|
Hi all, apologies for the delay. I've improved documentation in the areas pointed out; I tried to be as descriptive as possible while being concise. If more verbosity is needed let me know. |
|
@varunrajk this has never been merged, although approved. I see there is a conflict with your new changes tho. Can you fix this maybe? |
|
It has been a while, but I still think this would deserve to get merged |
3 participants
This introduces optimizations in the simulator's forward step that reduces the iteration time for sufficiently large populations. This is accomplished through
combinationandproductused for computing contact pairs to only the randomized indices that are relevantdictrather thanorderedSetswhere possible)Based on experiments, the execution time of running the simulator at the current
mainbranch vs these changes are as follows:Testing can be done by running
scripts/run_pandemic_sim.py