-
Notifications
You must be signed in to change notification settings - Fork 184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Multiprocess execution #82
base: master
Are you sure you want to change the base?
Conversation
I have run the exact same tests has travis in a couple of different machines without any problem... |
What's surprising is that both failed tests have exactly the same error |
The test is also OK on my laptop... Could this error come from a different library version ? Travis uses the most recent packages and we may have older ones. |
Looks like forcing to only use one process works for now... At least I had it run in travis twice with success. |
Hi @eSoares,
For your information, here is the multiprocessing I use, which gives much better speedup and scalability. Indeed, each core compute a whole SNR vs BER curve and then have more or less the same computation time. I use As explained earlier I have to turn off from commpy.links import link_performance
from p_tqdm import p_map
# Models as a list
models = []
# Define each model to test here...
# Helper function
def perf(model):
return link_performance(model, SNRs, nb_it, nb_err, chunk, code_rate)
BERs = p_map(perf, models, num_cpus=min(12, nb_test)) This is only my point of view. I am fully open to your opinions and arguments. |
Hello Bastian, I agree with some of your concerns.
I can try to set the seed inside each process, that should make the multiprocess reproducible and more stable. If this PR goes forward, I will make this change.
Did not know about this library, but looks a great improvement! Will use it in future! 👍
I guess the best strategy would be to parallelize the iterations inside
Well, that is up for debate if such elements should exist in the library or not. Even if they are not in the core part, at least in some examples it should exist.
For what I found is particularly problematic in matrix multiplication[1] (which is not used?), but the rest of operations were not problematic. But I'm no export in mixing numpy with multiprocess, so I take your statement and should be used with care :)
Thank you, is a nice library that I did not know of. 😃 About your example code in particular, that is kinda of what I do, but instead of parallelizing multiple models (since I only want to test one), I parallelize multiple SNRs (and MCS in the case of WiFi). In the end the question about "what makes sense to parallelize" is "depends what you're comparing" and of course "how many cores can you throw at it". If you have enough cores, parallelizing for each SNR may take more advantage of the available computing. Still even if this PR doesn't move forward, I think that at least an example and a set of instructions to be careful with numpy a multiprocess should result, for some less knowledgeable (as myself) don't make the mistakes you are pointing out! 😃 1 - https://stackoverflow.com/a/15647474 |
Do you think that it's related to the seed? Here are the results for the erroneous test from the Travis logs:
I agree that this should be somewhere either as a core element or as an example with guidelines. The latter looks better to me since this would be the place to explain the workaround with numpy's multithreading. However, adding multiprocessing as a core element is not irrational at all. @veeresht, do you have a long-term opinion?
I run into the problem when propagating through Lines 376 to 379 in 2a60fc5
Same here but I redirect the progress bar to a log file so that I have a costless insight on the computation time of each iterations.
If you test several MCS at once, it means that you're testing several models at once. In CommPy, a Anyway, if you have enough cores to parallize on each model and then still having some core available, you can split the workload on the # Create a tuple of models
models = (model,) * nb_core
# Helper function
def perf(model):
return link_performance(model, SNRs, nb_it // nb_core, nb_err // nb_core, chunk, code_rate)
# Split the computation and agregate the result
partial_BERs = p_map(perf, models)
np.array(partial_BERs).mean(0) |
Add a LinkModel that performs the simulation in multiprocess, one process per SNR.
The implementation works as a drop-in replacement, just need to import multiprocess_links instead of links.
Also adds a Wifi80211 multiprocess, where it can work one process per SNR, but can go further and perform one process per MCS x SNR. Example of usage available!
This multiplexing helps a lot, from the example available, running in the classic single process would take about 3min:7s while the multiprocess (under my 8 core work machine) takes about 40 seconds.
Because the limitations of the multiprocess in Python, any parameter to be passed to the process that executes the simulation need to be pickle serializable. This causes some problems when passing custom functions for receiving, decoding and so-on, but as long as they are in the root of the file and not nested (a function defined inside of a function) should work.