You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@systemds.apache.org by "ReneEnjilian (via GitHub)" <gi...@apache.org> on 2024/07/03 11:33:48 UTC
[PR] [SYSTEMDS-3172] Sparse MCSC Block [systemds]
ReneEnjilian opened a new pull request, #2040:
URL: https://github.com/apache/systemds/pull/2040
This pull request implements the Modified Compressed Sparse Column (MCSC) block. So far, most sparse blocks in SystemDS were row oriented (MCSR, CSR, DCSR). MCSC follows a similar design as MCSR but storing individual columns instead of rows. For simplicity, we reused the existing SparseRowVector/SparseRowScalar classes for the columns. The main contribution is in SparseBlockMCSC.java, where the entire structure is implemented. Further, testing has been added.
Subsequent tasks may require a deeper integration into the system. Specifically, a deeper integration is required regarding alignment, iterators and merging with other matrices.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SYSTEMDS-3172] Sparse MCSC Block [systemds]
Posted by "ReneEnjilian (via GitHub)" <gi...@apache.org>.
ReneEnjilian commented on PR #2040:
URL: https://github.com/apache/systemds/pull/2040#issuecomment-2208725942
> Hi @ReneEnjilian
>
> The tests does not show up in the code coverage because the new package of tests you added is not run in GitHub actions. to enable it simply add a new element in the list on:
>
> https://github.com/apache/systemds/blob/de91f669b87589d390a4c2804658210bb54ce1d7/.github/workflows/javaTests.yml#L57
>
> or add a new line with the elements if you know your tests take a long time (10 minutes +)
Hi @Baunsgaard,
thanks for your response. I am sorry but I maybe dont really understand you. I did not add a new package (meaning directory) of tests. These were existing tests and I just adapted them mostly. Do you want me to add **.component.s**.** in javaTests.yml?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SYSTEMDS-3172] Sparse MCSC Block [systemds]
Posted by "mboehm7 (via GitHub)" <gi...@apache.org>.
mboehm7 closed pull request #2040: [SYSTEMDS-3172] Sparse MCSC Block
URL: https://github.com/apache/systemds/pull/2040
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SYSTEMDS-3172] Sparse MCSC Block [systemds]
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #2040:
URL: https://github.com/apache/systemds/pull/2040#issuecomment-2208692571
Hi @ReneEnjilian
The tests does not show up in the code coverage because the new package of tests you added is not run in GitHub actions.
to enable it simply add a new element in the list on:
https://github.com/apache/systemds/blob/de91f669b87589d390a4c2804658210bb54ce1d7/.github/workflows/javaTests.yml#L57
or add a new line with the elements if you know your tests take a long time (10 minutes +)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SYSTEMDS-3172] Sparse MCSC Block [systemds]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2040:
URL: https://github.com/apache/systemds/pull/2040#issuecomment-2206015127
## [Codecov](https://app.codecov.io/gh/apache/systemds/pull/2040?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `0.37313%` with `267 lines` in your changes missing coverage. Please review.
> Please [upload](https://docs.codecov.com/docs/codecov-uploader) report for BASE (`main@8e1e53b`). [Learn more](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#section-missing-base-commit) about missing BASE report.
| [Files](https://app.codecov.io/gh/apache/systemds/pull/2040?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...org/apache/sysds/runtime/data/SparseBlockMCSC.java](https://app.codecov.io/gh/apache/systemds/pull/2040?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fsysds%2Fruntime%2Fdata%2FSparseBlockMCSC.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3N5c2RzL3J1bnRpbWUvZGF0YS9TcGFyc2VCbG9ja01DU0MuamF2YQ==) | 0.00% | [263 Missing :warning: ](https://app.codecov.io/gh/apache/systemds/pull/2040?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../apache/sysds/runtime/data/SparseBlockFactory.java](https://app.codecov.io/gh/apache/systemds/pull/2040?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fsysds%2Fruntime%2Fdata%2FSparseBlockFactory.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3N5c2RzL3J1bnRpbWUvZGF0YS9TcGFyc2VCbG9ja0ZhY3RvcnkuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/systemds/pull/2040?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../apache/sysds/runtime/matrix/data/MatrixBlock.java](https://app.codecov.io/gh/apache/systemds/pull/2040?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fsysds%2Fruntime%2Fmatrix%2Fdata%2FMatrixBlock.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3N5c2RzL3J1bnRpbWUvbWF0cml4L2RhdGEvTWF0cml4QmxvY2suamF2YQ==) | 0.00% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/systemds/pull/2040?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #2040 +/- ##
=======================================
Coverage ? 68.71%
Complexity ? 40487
=======================================
Files ? 1433
Lines ? 160779
Branches ? 31256
=======================================
Hits ? 110477
Misses ? 41294
Partials ? 9008
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/systemds/pull/2040?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SYSTEMDS-3172] Sparse MCSC Block [systemds]
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #2040:
URL: https://github.com/apache/systemds/pull/2040#issuecomment-2208761789
> > Hi @ReneEnjilian
> > The tests does not show up in the code coverage because the new package of tests you added is not run in GitHub actions. to enable it simply add a new element in the list on:
> > https://github.com/apache/systemds/blob/de91f669b87589d390a4c2804658210bb54ce1d7/.github/workflows/javaTests.yml#L57
> >
> > or add a new line with the elements if you know your tests take a long time (10 minutes +)
>
> Hi @Baunsgaard, thanks for your response. I am sorry but I maybe dont really understand you. I did not add a new package (meaning directory) of tests. These were existing tests and I just adapted them mostly. Do you want me to add **.component.s**.** in javaTests.yml?
no worries Matthias added it.
24a5724dce4d15c9d3d08f90dd76dc0d61b6b358
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SYSTEMDS-3172] Sparse MCSC Block [systemds]
Posted by "mboehm7 (via GitHub)" <gi...@apache.org>.
mboehm7 commented on PR #2040:
URL: https://github.com/apache/systemds/pull/2040#issuecomment-2208709012
LGTM - thanks @ReneEnjilian. During the merge I reverted the unnecessary `MatrixBlock` changes, fixed the warnings, simplified the tests, and added the sparse component tests to the github actions. Furthermore, I also fixed the java doc issues, of the previous failures on main.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org