-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Description
See the conversation here: #1580 (comment)
With the above change, I made it so the FastTree BinFinder.FindDistinctCounts was no longer destructive of the values VBuffer during CalculateBins.
Now that it no longer destroys the buffer, we no longer need to copy it here:
machinelearning/src/Microsoft.ML.FastTree/FastTree.cs
Lines 1475 to 1478 in cb9effc
| // Must copy over, as bin calculation is potentially destructive. | |
| copier(in temp, ref doubleTemp); | |
| hasMissing = !CalculateBins(finder, in doubleTemp, maxBins, 0, | |
| out BinUpperBounds[iFeature]); |
However, I couldn't easily remove this copy because doing the copy also changed the VBuffer from float to double. This should also be changed, as recognized by this REVIEW comment in the code:
| // REVIEW: Change this, as well as the bin finding code and bin upper bounds, to be Float instead of Double. |
This issue is to fix both of these things. First, change BinFinder to work on float instead of double. Then, we can remove this extra copy and just pass in the normal VBuffer<float> to BinFinder, without worrying if it will destroy the buffer.
/cc @Zruty0 @TomFinley