You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@systemml.apache.org by GitBox <gi...@apache.org> on 2020/03/29 08:10:13 UTC
[GitHub] [systemml] Baunsgaard opened a new pull request #871:
[SYSTEMDS-314] Python Federated Matrices
Baunsgaard opened a new pull request #871: [SYSTEMDS-314] Python Federated Matrices
URL: https://github.com/apache/systemml/pull/871
This commit contains:
- Easy start of federated worker in /bin/systemds.sh
- Setup of tests for the Python language bindings federated matrices
- Initial tutorial on Python federated matrices
- Minor :bug: fix in federated matrix, not allowing multiple sources
- Github workflow action for automated federated tests
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [systemml] Baunsgaard edited a comment on issue #871:
[SYSTEMDS-314] Python Federated Matrices
Posted by GitBox <gi...@apache.org>.
Baunsgaard edited a comment on issue #871: [SYSTEMDS-314] Python Federated Matrices
URL: https://github.com/apache/systemml/pull/871#issuecomment-605605198
Note, some tests are failing in python federated.
This is intentionally left in, so that we at some point can go back and fix the underlying functionality.
1. bug is if you create a federated matrix that is not 100% filled in by federated workers, then operations does not take into account the remaining cells.
2. bug is if you create a federated matrix that is larger than the input size, it crashes, while expected behavior is to pad the matrix with 0's for consistent behavior, with other functionality.
3. feature/bug, currently if a single federated matrix is having overlapping sub matrices, see tests ["federated basic test"](/src/main/python/tests/federated/test_federated_basic) for examples, the system does not complain, and in some cases even works by overwriting the matrix values of the first selected federated matrix with the later 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [systemml] Baunsgaard commented on issue #871: [SYSTEMDS-314]
Python Federated Matrices
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on issue #871: [SYSTEMDS-314] Python Federated Matrices
URL: https://github.com/apache/systemml/pull/871#issuecomment-605605198
Note, some tests are failing in python federated.
This is intentionally left in, so that we at some point can go back and fix the underlying functionality.
1. bug is if you create a federated matrix that is not 100% filled in by federated workers.
2. bug is if you create a federated matrix that is larger than the input size, it crashes, while expected behavior is to pad the matrix with 0's for consistent behavior.
3. feature/bug, currently if a single federated matrix is having overlapping sub matrices, see tests ["federated basic test"](/src/main/python/tests/federated/test_federated_basic) for examples, the system does not complain, and in some cases even works by overwriting the matrix values of the first selected federated matrix with the later 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [systemml] Baunsgaard commented on issue #871: [SYSTEMDS-314]
Python Federated Matrices
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on issue #871: [SYSTEMDS-314] Python Federated Matrices
URL: https://github.com/apache/systemml/pull/871#issuecomment-605601606
Also containing documentation update such that it is using relative paths to documentation.
This makes the references work in different forks, pointing to the forks docs.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [systemml] corepointer commented on a change in pull request #871:
[SYSTEMDS-314] Python Federated Matrices
Posted by GitBox <gi...@apache.org>.
corepointer commented on a change in pull request #871: [SYSTEMDS-314] Python Federated Matrices
URL: https://github.com/apache/systemml/pull/871#discussion_r400014472
##########
File path: src/main/python/tests/federated/runFedTest.sh
##########
@@ -0,0 +1,65 @@
+#/bin/bash
+#-------------------------------------------------------------
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#-------------------------------------------------------------
+
+if [ "$#" -ne 1 ]; then
+ echo "Usage: "$0" <federatedTest>"
+ echo "Example: "$0" ./tests/federated/test_federated_basic.py"
+ exit
+fi
+
+#FIELDS
+workerdir="tests/federated/worker/"
+outputdir="tests/federated/output/"
+mkdir $workerdir
+mkdir $outputdir
+w1_Output="$workerdir/w1"
+w2_Output="$workerdir/w2"
+log="$outputdir/out.log"
+
+# make the workers start quiertly
Review comment:
typo: quietly
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [systemml] Baunsgaard commented on a change in pull request #871:
[SYSTEMDS-314] Python Federated Matrices
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on a change in pull request #871: [SYSTEMDS-314] Python Federated Matrices
URL: https://github.com/apache/systemml/pull/871#discussion_r400051977
##########
File path: src/main/python/tests/federated/runFedTest.sh
##########
@@ -0,0 +1,65 @@
+#/bin/bash
+#-------------------------------------------------------------
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#-------------------------------------------------------------
+
+if [ "$#" -ne 1 ]; then
+ echo "Usage: "$0" <federatedTest>"
+ echo "Example: "$0" ./tests/federated/test_federated_basic.py"
+ exit
+fi
+
+#FIELDS
+workerdir="tests/federated/worker/"
+outputdir="tests/federated/output/"
+mkdir $workerdir
+mkdir $outputdir
+w1_Output="$workerdir/w1"
+w2_Output="$workerdir/w2"
+log="$outputdir/out.log"
+
+# make the workers start quiertly
Review comment:
Done!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [systemml] Baunsgaard commented on issue #871: [SYSTEMDS-314]
Python Federated Matrices
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on issue #871: [SYSTEMDS-314] Python Federated Matrices
URL: https://github.com/apache/systemml/pull/871#issuecomment-609071537
@mboehm7
I have commented out the "advanced" federated matrix tests.
This means it now verifies that the python federated basics work, and we can extend from 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [systemml] corepointer commented on a change in pull request #871:
[SYSTEMDS-314] Python Federated Matrices
Posted by GitBox <gi...@apache.org>.
corepointer commented on a change in pull request #871: [SYSTEMDS-314] Python Federated Matrices
URL: https://github.com/apache/systemml/pull/871#discussion_r400012824
##########
File path: docker/pythonsysds.Dockerfile
##########
@@ -0,0 +1,28 @@
+#-------------------------------------------------------------
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#-------------------------------------------------------------
+
+FROM openjdk:8-alpine
+
+RUN apk add --no-cache --virtual .build-deps g++ python3-dev libffi-dev openssl-dev && \
+ apk add --no-cache python3 && \
+ pip3 install --upgrade pip setuptools
+
+RUN pip3 install systemds
Review comment:
The github code browser widget suggests a newline at eof
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [systemml] Baunsgaard commented on issue #871: [SYSTEMDS-314]
Python Federated Matrices
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on issue #871: [SYSTEMDS-314] Python Federated Matrices
URL: https://github.com/apache/systemml/pull/871#issuecomment-605601427
It would be good to have a minor release with at least the federated worker startup part of this commit.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [systemml] Baunsgaard commented on a change in pull request #871:
[SYSTEMDS-314] Python Federated Matrices
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on a change in pull request #871: [SYSTEMDS-314] Python Federated Matrices
URL: https://github.com/apache/systemml/pull/871#discussion_r400045627
##########
File path: docker/pythonsysds.Dockerfile
##########
@@ -0,0 +1,28 @@
+#-------------------------------------------------------------
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#-------------------------------------------------------------
+
+FROM openjdk:8-alpine
+
+RUN apk add --no-cache --virtual .build-deps g++ python3-dev libffi-dev openssl-dev && \
+ apk add --no-cache python3 && \
+ pip3 install --upgrade pip setuptools
+
+RUN pip3 install systemds
Review comment:
Sure Thanks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [systemml] Baunsgaard commented on issue #871: [SYSTEMDS-314]
Python Federated Matrices
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on issue #871: [SYSTEMDS-314] Python Federated Matrices
URL: https://github.com/apache/systemml/pull/871#issuecomment-605904266
> LGTM except for the failing tests. In your comment, you mention that the failing tests are left in as a reminder to fix some bugs. Why not create a separate PR marked WIP or an issue in the tracker for that?
it makes sense to have tests fail showing the bugs, especially because it is functionality that we 'claim' to support in master.
i also think that @kev-inn would be suited for the fixing the bug. Therefore there i see two ways of going forward, either wait for that fix while being a pull request, or allow the pull request in that shows a bug in the system now. (i vote for the later (not that i have a vote) :smile: ).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [systemml] Baunsgaard commented on issue #871: [SYSTEMDS-315]
Python Federated Matrices
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on issue #871: [SYSTEMDS-315] Python Federated Matrices
URL: https://github.com/apache/systemml/pull/871#issuecomment-612881971
Fixed pull request to use the brand new SystemDSContext !! Thanks @kev-inn !
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services