[JVM-Package][WIP] Add missing value as parameter for DMatrix. #4954
Conversation
|
@CodingCat I got some kind help for Could you elaborate on why Scala needs to remove missing value itself? Need some more investigations, but still don't understand why Scala needs to handle missing value itself. |
|
@CodingCat I think I found the error, here this dataframe is passed as multiple data iterators: The first row is passed as a standalone iter (hence creating a DMatrix with single row), and Scala side removes the I think the primary reason of doing this is because creating xgboost/src/data/simple_csr_source.cc Line 73 in 010b8f1 |
|
I can't go all the way up to scala ... |
trivialfis
changed the title
trivialfis
changed the title
| * @param cacheInfo Cache path information, used for external memory setting, null by default. | ||
| * @throws XGBoostError native error | ||
| */ | ||
| def this(dataIter: Iterator[LabeledPoint], cacheInfo: String = null) { | ||
| this(new JDMatrix(dataIter.asJava, cacheInfo)) |
There was a problem hiding this comment.
instead of doing this, can we just add a new constructor?
| if (iter == null) { | ||
| throw new NullPointerException("iter: null"); | ||
| } | ||
| // 32k as batch size | ||
| int batchSize = 32 << 10; | ||
| Iterator<DataBatch> batchIter = new DataBatch.BatchIterator(iter, batchSize); | ||
| long[] out = new long[1]; | ||
| XGBoostJNI.checkCall(XGBoostJNI.XGDMatrixCreateFromDataIter(batchIter, cacheInfo, out)); | ||
| XGBoostJNI.checkCall(XGBoostJNI.XGDMatrixCreateFromDataIterEx( | ||
| batchIter, missing, cacheInfo, out)); |
There was a problem hiding this comment.
let's keep the original one and add new constructor
| @@ -48,6 +48,7 @@ class MissingValueHandlingSuite extends FunSuite with PerTest { | |||
| test("handle Float.NaN as missing value correctly") { | |||
| val spark = ss | |||
| import spark.implicits._ | |||
| println("handle Float.NaN as missing value correctly") | |||
|
@CodingCat So glad that you are here .. |
How do you want me to proceed? Should I simply add the missing value handling in c++ in this PR and remove other changes? There's a check in this PR that will fail the current missing value handling test in scala. |
Continue #4594 .