Makes sure we don't pull the whole corpus into memory when training #23
Conversation
| for (final List<List<String>> batch : partitioned) { | ||
| tasks.add(createWorker(i, iter, batch)); | ||
| for (final List<List<String>> batch : batched) { | ||
| futures.add(ex.submit(createWorker(i, iter, batch))); | ||
| i++; | ||
| } |
There was a problem hiding this comment.
This for loop would pull the entire training data into memory, because every worker contains a batch, and all workers are instantiated before the first one starts working.
|
Ping? |
| public NeuralNetworkModel train(final Iterable<List<String>> sentences) throws InterruptedException { | ||
| final ListeningExecutorService ex = | ||
| MoreExecutors.listeningDecorator( | ||
| new ThreadPoolExecutor(config.numThreads, config.numThreads, |
There was a problem hiding this comment.
Neat trick, but let's leave a comment to what we're trying to accomplish here. If I understand correctly, the overall idea is to have executor.submit block if there are no available threads to avoid materializing the sentences in memory before they are needed. The ArrayBlockingQueue and CallerRunsPolicy is one way to accomplish this.
Any reason why the blocking queue starts with size 8 instead of config.numThreads?
There was a problem hiding this comment.
That's correct. I'll add a comment.
The queue size could be config.numThreads, but it's not really connected to the number of processors. It's just connected to the amount of overhead there is in creating these threads. In principle, a queue size of 1 should do, but I tried that and it was slower. I'm worried that if I set it to the number of processors, I'll run out of memory on a machine with lots of cores.
There was a problem hiding this comment.
Actually, that's not correct. The queue size matters when the main thread is running a task due to the CallerRunsPolicy. So it is connected to the number of processors. I changed it.
|
This might be a fix for #20. |
|
Any info about then this pull request will be accepted? This changes helps me train 2,4 GB of data... |
|
The fix is really useful! It took us 70+ Gb of RAM to train a model without it. Now it's only about 10Gb. I wonder, why the improvement so essential hasn't yet been added to master? |
|
@wko27 had some concerns about the quality of the resulting vectors. @Hronom, @Iakovenko-Oleksandr, do you have any problems with your results? |
|
What kind of problems? It really feels like changed, but we still don't have any tools to evaluate adequacy of model... Closest vectors look more or less fine. |
Explanation in the comments.