Skip to content

Add sparse-data unit test for xvector-based linear regression #643

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.

Sign up for GitHub

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

Merged
highperformancecoder merged 6 commits into from
Apr 10, 2026
+33 −8
Merged

Add sparse-data unit test for xvector-based linear regression #643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5a1727b
Fix for linear regression calculation using the x-vector when data is…
highperformancecoder Apr 9, 2026
bbc5794
Address code review concerns
highperformancecoder Apr 9, 2026
bff0c30
Add unit test for sparse linear regression with NaN/Inf values in y w…
Copilot Apr 9, 2026
7868466
Mask out sumyy as well, per Coderabbit suggestion
highperformancecoder Apr 9, 2026
971e761
Merge branch 'bugfix/self-linear-regression-sparse-data' of github.co…
highperformancecoder Apr 9, 2026
1c7a046
Address code review comments
highperformancecoder Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions engine/minskyTensorOps.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1160,7 +1160,6 @@ namespace minsky
dimension=i-xv.begin();
}

sumy.setArgument(y,args);
TensorPtr spreadX;
if (x)
{
Expand All @@ -1169,7 +1168,7 @@ namespace minsky
}
else
{
if (rank()>1 && dimension>=y->rank()) return;
if (y->rank()>1 && dimension>=y->rank()) return;
// construct x from y's x-vector
auto tv=make_shared<TensorVal>();
spreadX=tv;
Expand All @@ -1194,13 +1193,17 @@ namespace minsky
}
}
}
sumx.setArgument(spreadX,args);
auto fxy=[](double x, double y){return isfinite(x) && isfinite(y)? x*y: 0;};
sumyy.setArgument(make_shared<BinOp>(fxy,y,y),args);
sumxx.setArgument(make_shared<BinOp>(fxy,spreadX,spreadX),args);
auto mask=[](double x, double y){return isfinite(x) && isfinite(y);};
auto fx=[mask](double x, double y){return mask(x,y)? x:0;};
auto fxy=[mask](double x, double y){return mask(x,y)? x*y: 0;};
auto maskedX=make_shared<BinOp>(fx,spreadX,y);
auto maskedY=make_shared<BinOp>(fx,y,spreadX);
sumx.setArgument(maskedX,args);
sumy.setArgument(maskedY,args);
sumyy.setArgument(make_shared<BinOp>(fxy,maskedY,y),args);
sumxx.setArgument(make_shared<BinOp>(fxy,maskedX,spreadX),args);
sumxy.setArgument(make_shared<BinOp>(fxy,y,spreadX),args);
count.setArgument
(make_shared<BinOp>([](double x,double y) {return isfinite(x)*isfinite(y);},y,spreadX),args);
count.setArgument(make_shared<BinOp>(mask,y,spreadX),args);
Comment on lines +1196 to +1206
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new masking logic (maskedX / fxy / count) changes linearRegression behavior for sparse inputs, but there doesn’t appear to be a unit test covering missing/non-finite y values when x is derived from y’s x-vector (the scenario this fix targets). Please add a gtest case (likely in test/testTensorOps.cc near the existing *LinearRegression tests) that includes NaN/Inf entries in y and verifies the regression output matches the expected line using only finite (x,y) pairs.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

@highperformancecoder highperformancecoder Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added xvectorValueLinearRegressionSparse test in test/testTensorOps.cc (commit bff0c30). The test creates a y-vector with xvector values {0,1,2,3,4,5} and data {1, NaN, 3, 4, Inf, 6}, then verifies the regression correctly identifies y = x + 1 using only the four finite pairs (0,1),(2,3),(3,4),(5,6).


assert(sumx.hypercube()==sumy.hypercube());
assert(sumx.index()==sumy.index());
Expand Down
22 changes: 22 additions & 0 deletions test/testTensorOps.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1844,6 +1844,28 @@ TEST_F(CorrelationSuite,xvectorValueLinearRegression)
for (size_t _i=0; _i<toVal.size(); ++_i) EXPECT_NEAR(result[_i], toVal[_i], 1e-4);
}

TEST_F(CorrelationSuite,xvectorValueLinearRegressionSparse)
{
// y = x + 1, but with NaN/Inf interspersed. Regression should use only finite (x,y) pairs.
Hypercube hc;
hc.xvectors.emplace_back("0", Dimension(Dimension::value,""), vector<civita::any>{0,1,2,3,4,5});
TensorVal y(hc); y=vector<double>{1, std::numeric_limits<double>::quiet_NaN(), 3, 4,
std::numeric_limits<double>::infinity(), 6};
fromVal=y;

// finite pairs: (x=0,y=1),(x=2,y=3),(x=3,y=4),(x=5,y=6) => line y=x+1
vector<double> result={1,2,3,4,5,6};

OperationPtr op(OperationType::linearRegression);
g->addItem(op);
Wire w1(from->ports(0),op->ports(1)), w3(op->ports(0),to->ports(1));
Eval(*to, op)();

auto& toVal=*to->vValue();
ASSERT_EQ(result.size(), toVal.size());
for (size_t _i=0; _i<toVal.size(); ++_i) EXPECT_NEAR(result[_i], toVal[_i], 1e-4);
}

TEST_F(CorrelationSuite,xvectorTimeLinearRegression)
{
Hypercube hc;
Expand Down
Loading