You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@systemds.apache.org by "ywcb00 (via GitHub)" <gi...@apache.org> on 2023/07/15 13:00:04 UTC
[GitHub] [systemds] ywcb00 opened a new pull request, #1863: [SYSTEMDS-2944] Federated Sparsity Propagation
ywcb00 opened a new pull request, #1863:
URL: https://github.com/apache/systemds/pull/1863
Hi,
This PR adds the functionality of propagating the sparsity from the federated workers to the coordinator when executing an instruction without subsequent data consolidation at the worker. To achieve this, the workers respond to a EXEC_INST request with the number of non-zeros of their respective federated partition. Thereby, we can obtain and aggregate the number of non-zeros and subsequently set the number of non-zeros of the (still federated) matrix object.
In this PR, several instructions have been extended to support the before mentioned sparsity propagation. I also added a respective unit test which compares the propagated number of non-zeros to a reference. Unfortunately, I had to copy parts of the _DMLScript.java_ code into the test class because the test needs direct access to the execution context of the script execution.
Additionally, I left three types of FIXME comments in the test dml script. In two of these comments, the underlying local operator (_ifelse_ and _cumsum_) returns an incorrect number of non-zeros for some reason that I haven't discovered yet. The other FIXME comment relates to an incorrect computation of instructions like _sum(X!=0)_ which is introduced by rewriting the constructed HOP.
NOTE: This PR is based on #1862 . Hence, we should wait with the merge until the other PR has been resolved.
Thanks for review :)
--
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
[GitHub] [systemds] Baunsgaard commented on pull request #1863: [SYSTEMDS-2944] Federated Sparsity Propagation
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1863:
URL: https://github.com/apache/systemds/pull/1863#issuecomment-1646653013
> Thanks for the review @Baunsgaard :) I also like that aspect of the tests. However, I don't like the copied code parts from _DMLScript_. Any idea how to prevent this code duplication? Or do you think that it's fine like this?
If you want to avoid the duplicate maybe extending the class can help.
Unfortunately sometimes the easy way is to duplicate some code in the testing. And it did not seem excessive in this case.
--
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
[GitHub] [systemds] mboehm7 commented on pull request #1863: [SYSTEMDS-2944] Federated Sparsity Propagation
Posted by "mboehm7 (via GitHub)" <gi...@apache.org>.
mboehm7 commented on PR #1863:
URL: https://github.com/apache/systemds/pull/1863#issuecomment-1650203105
LGTM too - I'll merge it in and fix the remaining issues in the process.
--
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
[GitHub] [systemds] ywcb00 commented on pull request #1863: [SYSTEMDS-2944] Federated Sparsity Propagation
Posted by "ywcb00 (via GitHub)" <gi...@apache.org>.
ywcb00 commented on PR #1863:
URL: https://github.com/apache/systemds/pull/1863#issuecomment-1646334927
Thanks for the review @Baunsgaard :)
I also like that aspect of the tests. However, I don't like the copied code parts from _DMLScript_. Any idea how to prevent this code duplication? Or do you think that it's fine like this?
--
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
[GitHub] [systemds] mboehm7 closed pull request #1863: [SYSTEMDS-2944] Federated Sparsity Propagation
Posted by "mboehm7 (via GitHub)" <gi...@apache.org>.
mboehm7 closed pull request #1863: [SYSTEMDS-2944] Federated Sparsity Propagation
URL: https://github.com/apache/systemds/pull/1863
--
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
[GitHub] [systemds] Baunsgaard commented on pull request #1863: [SYSTEMDS-2944] Federated Sparsity Propagation
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1863:
URL: https://github.com/apache/systemds/pull/1863#issuecomment-1641758033
Also, David,
It seems like you did not finish your signing up to Apache. since your user does not show you are part of the Apache group. Maybe you want to do that ?
--
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