Skip to content

[SYSTEMDS-3948] Row-wise Sparsity Estimator#2466

Closed
ywcb00 wants to merge 16 commits into
apache:mainfrom
ywcb00:feat/sparsity/estim/rs
Closed

[SYSTEMDS-3948] Row-wise Sparsity Estimator#2466
ywcb00 wants to merge 16 commits into
apache:mainfrom
ywcb00:feat/sparsity/estim/rs

Conversation

@ywcb00
Copy link
Copy Markdown
Contributor

@ywcb00 ywcb00 commented May 6, 2026

Hi,
this PR adds the row-wise sparsity estimator from:
Lin, Chunxu, Wensheng Luo, Yixiang Fang, Chenhao Ma, Xilin Liu and Yuchi Ma;
On Efficient Large Sparse Matrix Chain Multiplication;
Proceedings of the ACM on Management of Data 2 (2024): 1 - 27.

Note that the row sparsity propagation, as described in the publication, applies to MM chains only. Other operations use fallback methods for sparsity estimation w/ row sparsity vectors, which create a cut in the sparsity estimation DAG.

Copy link
Copy Markdown
Contributor

@Baunsgaard Baunsgaard left a comment

Choose a reason for hiding this comment

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

Some comments. hope it helps.

Comment thread src/test/java/org/apache/sysds/test/component/estim/OpSingleTest.java Outdated
Comment thread src/test/java/org/apache/sysds/test/component/estim/OpSingleTest.java Outdated
Comment thread src/main/java/org/apache/sysds/hops/estim/EstimatorRowWise.java Outdated
Comment thread src/main/java/org/apache/sysds/hops/estim/EstimatorRowWise.java Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 11, 2026

Codecov Report

❌ Patch coverage is 74.83871% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.38%. Comparing base (249a8f5) to head (73dd10b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
.../org/apache/sysds/hops/estim/EstimatorRowWise.java 74.83% 25 Missing and 14 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2466      +/-   ##
============================================
+ Coverage     71.36%   71.38%   +0.01%     
- Complexity    48684    48751      +67     
============================================
  Files          1570     1571       +1     
  Lines        188757   188912     +155     
  Branches      37039    37067      +28     
============================================
+ Hits         134712   134849     +137     
- Misses        43591    43601      +10     
- Partials      10454    10462       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ywcb00 ywcb00 force-pushed the feat/sparsity/estim/rs branch 2 times, most recently from 1f2c869 to 30cc518 Compare May 15, 2026 13:39
@ywcb00
Copy link
Copy Markdown
Contributor Author

ywcb00 commented May 15, 2026

Thank you for the comments @Baunsgaard . I have updated the PR accordingly. :)

Copy link
Copy Markdown
Contributor

@Baunsgaard Baunsgaard left a comment

Choose a reason for hiding this comment

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

A few comments

Comment thread src/main/java/org/apache/sysds/hops/estim/EstimatorRowWise.java Outdated
Comment thread src/main/java/org/apache/sysds/hops/estim/EstimatorRowWise.java Outdated
Comment thread src/main/java/org/apache/sysds/hops/estim/EstimatorRowWise.java Outdated
Comment thread src/main/java/org/apache/sysds/hops/estim/EstimatorRowWise.java Outdated
Comment thread src/main/java/org/apache/sysds/hops/estim/EstimatorRowWise.java Outdated
Comment thread src/main/java/org/apache/sysds/hops/estim/EstimatorRowWise.java Outdated
@ywcb00
Copy link
Copy Markdown
Contributor Author

ywcb00 commented May 19, 2026

Thank you for the valuable comments. :)

Copy link
Copy Markdown
Contributor

@Baunsgaard Baunsgaard left a comment

Choose a reason for hiding this comment

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

PR LGTM, there are only micro optimizations to do now.

Comment thread src/main/java/org/apache/sysds/hops/estim/EstimatorRowWise.java Outdated
Comment thread src/main/java/org/apache/sysds/hops/estim/EstimatorRowWise.java Outdated
Comment thread src/main/java/org/apache/sysds/hops/estim/EstimatorRowWise.java
ywcb00 added 16 commits May 20, 2026 15:34
…ise sparsity estimator

	works for the matrix multiplication and bind test cases for now
…ontainer for row wise sparsity vectors to simplify access and allow storing it with chain nodes
…mator with element-wise and single operations
…wise and single operations

	NOTE: using average case estimation per row
…ro and diag (mv and vm) operations with the row-wise sparsity estimator
… to consolidate all calls to getters before the switch
… for row-wise sparsity vector and apply the corresponding operations directly in the code instead
…ts to double type for constant values

	remove comment stating that this would be the best estimate for the respective case
@ywcb00 ywcb00 force-pushed the feat/sparsity/estim/rs branch from 04fcd6f to 73dd10b Compare May 20, 2026 13:34
@ywcb00 ywcb00 closed this in c50c769 May 21, 2026
@github-project-automation github-project-automation Bot moved this from In Progress to Done in SystemDS PR Queue May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants