Skip to content

Change FastTree BinFinder to use floats and remove one data copy #1648

Open
@eerhardt

Description

@eerhardt

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:

// 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

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Priority of the issue for triage purpose: Needs to be fixed at some point.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions