Please support NULL values

Describe the bug
Null is basically unsupported. It is hard to filter nulls and much harder to grasp when exactly null is being substituted by default(T).

To Reproduce
Nuget Micorsoft.Data.Analysis 0.20.1:

var coln = new Int32DataFrameColumn("test", new int?[] { -2, null });
var colp = new Int32DataFrameColumn("test2", new int?[] { 2, null });

Console.WriteLine($"linq max: {Enumerable.Max(coln)} mda column max: {coln.Max()}");
Console.WriteLine($"linq avg: {Enumerable.Average(coln)} mda column avg: {coln.Mean()}");
Console.WriteLine($"linq min: {Enumerable.Min(colp)} mda column min: {colp.Min()}");

// output
linq max: -2 mda column max: 0
linq avg: -2 mda column avg: -1
linq min: 2 mda column min: 0

Expected behavior
I would expect most of Dataframe, here Min/Max/Mean, to behave like SQL and LINQ do with regard to NULL values.

@simonthum

The bad news is that's embarrassing :)

The good news is that it's actually all generated from a TT file - so it should be fairly easy to update all of them.

https://github.com/dotnet/machinelearning/blob/main/src/Microsoft.Data.Analysis/PrimitiveDataFrameColumnComputations.tt

Would you be up for giving it a shot? @simonthum

I'm happy to review the PR. I'm planning to investigate a bunch of DataFrame bugs but I'm not sure when I'll get to this one.

Related Issue: #6144

I'd probably try to get it to generate something like this:

public void Sum(PrimitiveColumnContainer<int> column, out int ret)
        {
            ret = (int)0;
            for (int b = 0; b < column.Buffers.Count; b++)
            {

                var readOnlySpan = column.Buffers[b].ReadOnlySpan;
                var readOnlyBitMapSpan = column.NullBitMapBuffers[b].ReadOnlySpan;

                for (int i = 0; i < readOnlySpan.Length; i++)
                {
                    if (column.IsValid(readOnlyBitMapSpan, i))
                    {
                        ret = (int)(readOnlySpan[i] + ret);
                    }
                }
            }
        }

The IsValid call checks if it's null and excludes it from the sum.

I think I made the needed changes in #6661 but it would need a bunch of tests added.

I specifically don't know what the cumulative functions should do with nulls.

If the old value in that spot was null should the new value be null. In my code it isn't because the cumulative to that point is never null.

Thanks for addressing this! I was just trying it out to see what workloads are in range with the mda dataframe. I have to confess I was slightly disappointed, although performace was good.

AFAICT in SQL most functions yield NULL when there is no non-null input, and treat NULL input as if nothing happened. Maybe that clarifies the cumulative case? In any case I would align to SQL where sensible.

But as you say, to keep embarassing stuff from popping up the whole API (not just math) needs to be reviewed and probably deserves null/0 test cases. For example, sum may return both null or 0, they are truly different cases.

@simonthum

Closing this issue since the PR to address this issue has already been merged. Thanks @simonthum

Navigation Menu

Repository navigation

Please support NULL values #6659

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Please support NULL values #6659

Description

Activity

JakeRadMSFT commented on May 6, 2023

JakeRadMSFT commented on May 6, 2023

JakeRadMSFT commented on May 6, 2023

simonthum commented on May 8, 2023

luisquintanilla commented on May 10, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions