Skip to content
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

base_margin not copied by XGDMatrixSliceDMatrix. Leads to loss of base_margin when performing cross-validation with xgb.cv() #2006

Closed
osofr opened this issue Feb 4, 2017 · 5 comments · Fixed by #2007

Comments

@osofr
Copy link
Contributor

@osofr osofr commented Feb 4, 2017

On latest version of the R package.

base_margin is not passed down to test / train data during cross-validation.

On the other hand other setinfo attributes such as weights and labels are passed down and handled properly (split into test / train sets).

Most likely the problem occurs when calling slice in xgb.cv, which can be traced down to these lines, where we can see that weights and labels are copied, but not base_margin. Presumabely this is a very quick fix, would it be possible to add a line to copy base_margin?

xgboost/src/c_api/c_api.cc

Lines 397 to 405 in 7078c41

if (src.info.labels.size() != 0) {
ret.info.labels.push_back(src.info.labels[ridx]);
}
if (src.info.weights.size() != 0) {
ret.info.weights.push_back(src.info.weights[ridx]);
}
if (src.info.root_index.size() != 0) {
ret.info.root_index.push_back(src.info.root_index[ridx]);
}

@tqchen

This comment has been minimized.

Copy link
Member

@tqchen tqchen commented Feb 4, 2017

hmm, can you open a PR to fix this? Thanks!

@osofr

This comment has been minimized.

Copy link
Contributor Author

@osofr osofr commented Feb 4, 2017

I can try, but I hardly know any C++ :)

@owlas

This comment has been minimized.

Copy link
Contributor

@owlas owlas commented Oct 16, 2017

I just ran into this in the current version on pypi.

  • Is this fix in the most recent release?
  • Why was the issue reopened?
@khotilov

This comment has been minimized.

Copy link
Member

@khotilov khotilov commented Oct 17, 2017

It was fixed in the development but so far it got into neither pypi nor CRAN release.

@khotilov khotilov closed this Oct 17, 2017
@owlas

This comment has been minimized.

Copy link
Contributor

@owlas owlas commented Oct 17, 2017

@khotilov thanks. Is there a schedule for next pypi release?

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.