search

prioritizr / benchmark

Benchmark performance of exact algorithms solvers for conservation planning
GNU General Public License v3.0
0 stars 0 forks source link

lpsymphony fork bomb #5

Closed jeffreyhanson closed 7 months ago

jeffreyhanson commented 3 years ago

The lpsymphony R package will by default use all available threads on the system. This means that if we try running the benchmarks using parallel processing (i.e. multiple runs at once) and we include add_lpsymphony_solver then this will fork bomb the system (i.e. R will try to use more threads than cores on the system). To address this, we can (1) disable parallel processing for the benchmarks, (2) exclude lpsymphony from the benchmarks, or (3) see if lpsymphony can be updated to include a parameter to specify the number of threads. Or maybe something else?

@ricschuster, I'll make an issue on its GitHub repo and to see if the maintainer would be open to a PR for addressing (3). In the meantime, what do you think we should do?

ricschuster commented 3 years ago

I'd vote for (1) disable parallel processing. Might be the fairest comparison between solvers?

I do like to throw threads at things, but maybe best to keep benchmarking to single thread.

jeffreyhanson commented 3 years ago

Ok sounds good. I'll update the parameter file to disable benchmarks runs in parallel.

jeffreyhanson commented 3 years ago

For reference, I've created the issue here: https://github.com/vladchimescu/lpsymphony/issues/6

jeffreyhanson commented 1 year ago

@ricschuster, it looks like the benchmarks will still be plauged by lpsymphony fork bombs (because it will try running a bunch of runs simulaneously with 1 thread each, and then running lpsymphony which will create N threads where N == number of cores). I'll quickly updated the PR to adress this.

jeffreyhanson commented 1 year ago

@ricschuster, just FYI, found some bugs in the highs solver code when re-running the test benchmarks. So might be a while before I can get it reasdy for you.

ricschuster commented 1 year ago

Thanks for the update. I will set everything up on my end and wait to hear from you before running anything.

jeffreyhanson commented 7 months ago

This issue has been fixed, so closing it now