You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@systemds.apache.org by GitBox <gi...@apache.org> on 2022/11/03 17:05:12 UTC
[GitHub] [systemds] sebwrede opened a new pull request, #1721: Docker Setup For HE
sebwrede opened a new pull request, #1721:
URL: https://github.com/apache/systemds/pull/1721
This PR changes the dockerfile for tests to include SEAL for running the homomorphic encryption tests. Currently, the HE tests are ignored, but this PR removes the 'ignore' tags and adds the tests to the GitHub test workflows.
--
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] sebwrede commented on a diff in pull request #1721: [SYSTEMDS-3280] Docker Setup For HE
Posted by GitBox <gi...@apache.org>.
sebwrede commented on code in PR #1721:
URL: https://github.com/apache/systemds/pull/1721#discussion_r1013702132
##########
docker/entrypoint.sh:
##########
@@ -22,6 +22,8 @@
# A script to execute the tests inside the docker container.
+cd /github/workspace/src/main/cpp
+./build.sh
Review Comment:
It takes around 20 seconds. We anyway build SystemDS every time, so I don't think it is out of place to build the dependencies first.
--
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 #1721: [SYSTEMDS-3280] Docker Setup For HE
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on PR #1721:
URL: https://github.com/apache/systemds/pull/1721#issuecomment-1304813682
> > Do you know how to update the docker image ?
>
> Not really. Do I even have permissions to update it?
as a committer you have the ability to build the images:
Go to actions
Then Docker release deployment action (for normal docker image)
Or For test image use Docker Test Image Update.
Then run workflow.
Note that currently you have to have the code on master for tit to work.
--
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] sebwrede commented on pull request #1721: [SYSTEMDS-3280] Docker Setup For HE
Posted by GitBox <gi...@apache.org>.
sebwrede commented on PR #1721:
URL: https://github.com/apache/systemds/pull/1721#issuecomment-1303292527
> Do you know how to update the docker image ?
Not really. Do I even have permissions to update it?
--
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] sebwrede commented on a diff in pull request #1721: [SYSTEMDS-3280] Docker Setup For HE
Posted by GitBox <gi...@apache.org>.
sebwrede commented on code in PR #1721:
URL: https://github.com/apache/systemds/pull/1721#discussion_r1013698831
##########
src/test/java/org/apache/sysds/test/functions/federated/paramserv/EncryptedFederatedParamservTest.java:
##########
@@ -73,7 +72,7 @@ public static Collection<Object[]> parameters() {
//{"TwoNN", 5, 1000, 100, 1, 0.01, "BSP", "BATCH", "KEEP_DATA_ON_WORKER", "NONE", "true", "BALANCED", 200},
{"TwoNN", 2, 4, 1, 4, 0.01, "SBP", "BATCH", "KEEP_DATA_ON_WORKER", "BASELINE", "false", "IMBALANCED", 200},
{"TwoNN", 2, 4, 1, 4, 0.01, "SBP", "BATCH", "KEEP_DATA_ON_WORKER", "BASELINE", "false", "BALANCED", 200},
- {"CNN", 2, 4, 1, 4, 0.01, "SBP", "EPOCH", "SHUFFLE", "BASELINE", "false", "BALANCED", 200},
+ //{"CNN", 2, 4, 1, 4, 0.01, "SBP", "EPOCH", "SHUFFLE", "BASELINE", "false", "BALANCED", 200},
Review Comment:
It cannot shuffle when it is encrypted. The CNN works.
--
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 #1721: [SYSTEMDS-3280] Docker Setup For HE
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on PR #1721:
URL: https://github.com/apache/systemds/pull/1721#issuecomment-1303248784
Lgtm.
Do you know how to update the docker image ?
--
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 a diff in pull request #1721: [SYSTEMDS-3280] Docker Setup For HE
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on code in PR #1721:
URL: https://github.com/apache/systemds/pull/1721#discussion_r1013306627
##########
docker/entrypoint.sh:
##########
@@ -22,6 +22,8 @@
# A script to execute the tests inside the docker container.
+cd /github/workspace/src/main/cpp
+./build.sh
Review Comment:
i do not like this, since it would mean we have to build our c++ part every test.
do you know how much the overhead is?
##########
src/test/java/org/apache/sysds/test/functions/federated/paramserv/EncryptedFederatedParamservTest.java:
##########
@@ -73,7 +72,7 @@ public static Collection<Object[]> parameters() {
//{"TwoNN", 5, 1000, 100, 1, 0.01, "BSP", "BATCH", "KEEP_DATA_ON_WORKER", "NONE", "true", "BALANCED", 200},
{"TwoNN", 2, 4, 1, 4, 0.01, "SBP", "BATCH", "KEEP_DATA_ON_WORKER", "BASELINE", "false", "IMBALANCED", 200},
{"TwoNN", 2, 4, 1, 4, 0.01, "SBP", "BATCH", "KEEP_DATA_ON_WORKER", "BASELINE", "false", "BALANCED", 200},
- {"CNN", 2, 4, 1, 4, 0.01, "SBP", "EPOCH", "SHUFFLE", "BASELINE", "false", "BALANCED", 200},
+ //{"CNN", 2, 4, 1, 4, 0.01, "SBP", "EPOCH", "SHUFFLE", "BASELINE", "false", "BALANCED", 200},
Review Comment:
i assume the CNN did not work?
--
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] sebwrede commented on pull request #1721: [SYSTEMDS-3280] Docker Setup For HE
Posted by GitBox <gi...@apache.org>.
sebwrede commented on PR #1721:
URL: https://github.com/apache/systemds/pull/1721#issuecomment-1303080309
> Can we perhaps make a different docker file that contains the homomorphic encryption and not the R libs maybe, and then build inside this GitHub action?
How can we make sure that we don't need the R libs for the HE tests? Currently it may be true, but what if we add some other tests later on where both HE and R libs are needed?
Additionally, I think it would be a bit confusing if you need two different Docker images to run all tests instead of having a single Docker image to run all tests.
Or did you mean that we should then include the HE dockerfile in the existing test dockerfile so that we still build a single image, but just have the configurations in two separate files?
--
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] sebwrede closed pull request #1721: [SYSTEMDS-3280] Docker Setup For HE
Posted by GitBox <gi...@apache.org>.
sebwrede closed pull request #1721: [SYSTEMDS-3280] Docker Setup For HE
URL: https://github.com/apache/systemds/pull/1721
--
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