Fix external memory, gpu_hist and subsampling combination bug. (#7476) #7481
Conversation
|
|
||
| // Compact the ELLPACK pages into the single sample page. | ||
| thrust::fill(dh::tbegin(page_->gidx_buffer), dh::tend(page_->gidx_buffer), 0); | ||
| for (auto& batch : dmat->GetBatches<EllpackPage>(batch_param_)) { | ||
| for (auto& batch : batch_iterator) { |
There was a problem hiding this comment.
Is it possible that the first page is being duplicated?
There was a problem hiding this comment.
Do you mean the new constructor called with first_page data and then again using the same page in the iterator?
If that is the question, then I think the constructor only uses first_page to get some info on how to reserve memory for page_. Later, in the iterator, the real processing is performed so first_page must appear again there.
- This commit refers to the suggestion dmlc#7481 (review) - Adds a test that accompanies the fix dmlc#7476, the test segfaults before the commit dmlc#7481.
I've added the test you suggested. Just copied the existing code from run_data_iterator test into a new test, and added subsample value less than 1. Cheers! |
|
Let me take a closer look and fix the test |
|
|
Thank you for the nice work. I will get back to this tomorrow. I don't have access to my device at the moment. |
|
Ok, now I understand what you meant when you told me to add test. Nice work, thank you. I just added the relaxing of rtol when subsample < 1.0. This is the main reason for test fail. I added scaled rtol, lets see if it will pass. |
|
Will take a closer look. The quantile is created differently with batched data, so testing prediction result is not reliable. |
|
Opened an issue for the major cause of floating-point error: #7488 Will follow up next week. (I'm currently out of the office, sorry). |
…7476) - The error happens because when reading from external memory the batch is reassembled for every new iteration. The variable `original_page_` is initialized from the first batch, when the constructor of `GradiendBasedSample` is called. After iterating through the batches the original memory is not accessible, so when trying to access the memory pointed by `original_page_` causes an error. - The solution is instead of accessing data from the `original_page_`, to access the data from the first page of the available batch. fix dmlc#7476
- This commit refers to the suggestion dmlc#7481 (review) - Adds a test that accompanies the fix dmlc#7476, the test segfaults before the commit dmlc#7481.
- This test fails when run on CPU, so only on GPU should be run. - Added sampling method testing.
trivialfis
force-pushed
the
fix-external-gpu_hist
branch
from
da40315
to
88d0bff
Compare
|
Thanks for the fix and sorry for the long delay. |
The error happens because when reading from external memory the batch is
reassembled for every new iteration. The variable
original_page_isinitialized from the first batch, when the constructor of
GradiendBasedSampleis called. After iterating through the batches the original memory is not
accessible, so when trying to access the memory pointed by
original_page_causes an error.
The solution is instead of accessing data from the
original_page_, to accessthe data from the first page of the available batch.
fix #7476