search

dmlc / XGBoost.jl

XGBoost Julia Package
Other
288 stars 110 forks source link

fix horrifying predict output transpose issue #144

Closed ExpandingMan closed 1 year ago

ExpandingMan commented 1 year ago

This fixes #143 in which the output of predict, if a matrix, would get scrambled because it failed to transpose the dimensions returned by the row-major libxboost. Unfortunately this comes at the cost of the type stability of predict since now the output may be a transpose array (though we assume this is preferable to actually copying it).

I have added a test for classifications that return matrix output from predict using test data that I generated with MLJ. This data has been added to assets/data.

Since this is a critical issue I'm probably not going to wait very long to merge this.

bobaronoff commented 1 year ago

Thank you for addressing this so promptly. I am new to this so pardon if this is a silly comment. Noticed in your code you added a simple transpose if the return length(dim)>1. I don't think a simple transpose will suffice. I had to reshape the matrix before transposing. For example the 'old' return matrix was (920,5) - a column for each class. That is correct dimension. A transpose will give a (5,920) matrix - not helpful. I first needed to reshape the (920,5) to (5,920) and then apply the transpose. I think this is correct but apologize if I am wrong.

regards, Bob

ExpandingMan commented 1 year ago

There's a transpose but it also reverses the dimensions that are output to be used in the matrix constructor. That's why you weren't able to fix it with a transpose alone.