Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PrimitiveDataFrameColumnComputations produce wrong result for Min/Max functions #5759

Open
Tracked by #6144
0xcafebabee opened this issue Apr 22, 2021 · 2 comments
Open
Tracked by #6144
Labels
Microsoft.Data.Analysis All DataFrame related issues and PRs

Comments

@0xcafebabee
Copy link
Contributor

System information

  • OS version/distro: Windows 10
  • .NET Version (eg., dotnet --info): netstandard2.0

Issue

Microsoft.Data.Analysis\PrimitiveDataFrameColumnComputations.cs provide wrong result for Min/Max functions

For these functions (and probably some other) we have wrong computation results:

We start computing from default value (code produced from tt templates):

Default value for integer is 0 and on each iteration we set 0 to ret not regarding that I have (for example) only positive values in column container:

public void Min(PrimitiveColumnContainer column, IEnumerable rows, out int ret)
{
ret = default;
var readOnlySpan = column.Buffers[0].ReadOnlySpan;
long minRange = 0;
long maxRange = ReadOnlyDataFrameBuffer.MaxCapacity;
long maxCapacity = maxRange;
IEnumerator enumerator = rows.GetEnumerator();
while (enumerator.MoveNext())
{
long row = enumerator.Current;
if (row < minRange || row >= maxRange)
{
int bufferIndex = (int)(row / maxCapacity);
readOnlySpan = column.Buffers[bufferIndex].ReadOnlySpan;
minRange = checked(bufferIndex * maxCapacity);
maxRange = checked((bufferIndex + 1) * maxCapacity);
}
row -= minRange;
ret = (int)(Math.Min(readOnlySpan[(int)row], ret));
}
}

As a result I have ret = 0. The same case is in Max function if I have only negative values in readOnlySpan collection.

@ericstj ericstj added the Microsoft.Data.Analysis All DataFrame related issues and PRs label Apr 22, 2021
@ericstj
Copy link
Member

ericstj commented Apr 22, 2021

cc @pgovind @eerhardt

@asmirnov82
Copy link
Contributor

Hello @luisquintanilla, looks like this issues is already fixed by #6147 and may be closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Microsoft.Data.Analysis All DataFrame related issues and PRs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants