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/04/23 12:32:09 UTC
[GitHub] [systemml] Baunsgaard opened a new pull request #896: [MINOR] Python Lineage Trace Test Change
Baunsgaard opened a new pull request #896:
URL: https://github.com/apache/systemml/pull/896
[MINOR] Python Lineage Trace Test Change
Change to the way lineage trace tests are executed, such that instead of
having files to compare to, the python trace is compared to a trace made
from systemds directly.
Motivated by the fact that the previous tests were failing, because of
inconsistencies between new traces and old.
----------------------------------------------------------------
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
[GitHub] [systemml] Baunsgaard commented on pull request #896: [MINOR] Python Lineage Trace Test Change
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on pull request #896:
URL: https://github.com/apache/systemml/pull/896#issuecomment-618872315
LGTM :grinning:
@mboehm7 or @j143 , could you merge ? (It would fix the few issues we have on master now.)
----------------------------------------------------------------
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
[GitHub] [systemml] Baunsgaard commented on issue #896: [MINOR] Python Lineage Trace Test Change
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on issue #896:
URL: https://github.com/apache/systemml/pull/896#issuecomment-618375500
Currently failing because python interface call operations with 1 thread only, while sysds calls with max number of threads.
----------------------------------------------------------------
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
[GitHub] [systemml] mboehm7 commented on pull request #896: [MINOR] Python Lineage Trace Test Change
Posted by GitBox <gi...@apache.org>.
mboehm7 commented on pull request #896:
URL: https://github.com/apache/systemml/pull/896#issuecomment-619157501
LGTM - thanks @Baunsgaard. In general this is fine, but in the future, it would be good to preserve the history for renamed/moved files (`src/main/python/tests/test_lineagetrace.py` to
`src/main/python/tests/lineage/test_lineagetrace.py`). What usually works is to first rename it, commit, and make the necessary changes in a subsequent 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
[GitHub] [systemml] Baunsgaard commented on a change in pull request #896: [MINOR] Python Lineage Trace Test Change
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on a change in pull request #896:
URL: https://github.com/apache/systemml/pull/896#discussion_r413777910
##########
File path: src/main/python/tests/lineage/test_lineagetrace.py
##########
@@ -0,0 +1,98 @@
+# -------------------------------------------------------------
+#
+# 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.
+#
+# -------------------------------------------------------------
+
+import warnings
+import unittest
+import os
+import shutil
+import sys
+import re
+
+path = os.path.join(os.path.dirname(os.path.realpath(__file__)), "../../")
+sys.path.insert(0, path)
+from systemds.context import SystemDSContext
+
+sds = SystemDSContext()
+
+
+class TestLineageTrace(unittest.TestCase):
+ def setUp(self):
+ warnings.filterwarnings(
+ action="ignore", message="unclosed", category=ResourceWarning
+ )
+
+ def tearDown(self):
+ warnings.filterwarnings(
+ action="ignore", message="unclosed", category=ResourceWarning
+ )
+
+ def test_compare_trace1(self): # test getLineageTrace() on an intermediate
+ m = sds.full((10, 10), 1)
+ m_res = m + m
+
+ python_trace = [x.strip() for x in m_res.get_lineage_trace().split("\n")]
Review comment:
Do we really want the trace to be returned as one large string as default, or something like a list?
----------------------------------------------------------------
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
[GitHub] [systemml] Baunsgaard commented on issue #896: [MINOR] Python Lineage Trace Test Change
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on issue #896:
URL: https://github.com/apache/systemml/pull/896#issuecomment-618375842
@phaniarnab comments, please?
----------------------------------------------------------------
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
[GitHub] [systemml] j143 commented on pull request #896: [MINOR] Python Lineage Trace Test Change
Posted by GitBox <gi...@apache.org>.
j143 commented on pull request #896:
URL: https://github.com/apache/systemml/pull/896#issuecomment-619429199
Normally, the `refactor` option in the IDEs should preserve git history.
----------------------------------------------------------------
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
[GitHub] [systemml] phaniarnab commented on a change in pull request #896: [MINOR] Python Lineage Trace Test Change
Posted by GitBox <gi...@apache.org>.
phaniarnab commented on a change in pull request #896:
URL: https://github.com/apache/systemml/pull/896#discussion_r413905307
##########
File path: src/main/python/tests/lineage/test_lineagetrace.py
##########
@@ -0,0 +1,98 @@
+# -------------------------------------------------------------
+#
+# 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.
+#
+# -------------------------------------------------------------
+
+import warnings
+import unittest
+import os
+import shutil
+import sys
+import re
+
+path = os.path.join(os.path.dirname(os.path.realpath(__file__)), "../../")
+sys.path.insert(0, path)
+from systemds.context import SystemDSContext
+
+sds = SystemDSContext()
+
+
+class TestLineageTrace(unittest.TestCase):
+ def setUp(self):
+ warnings.filterwarnings(
+ action="ignore", message="unclosed", category=ResourceWarning
+ )
+
+ def tearDown(self):
+ warnings.filterwarnings(
+ action="ignore", message="unclosed", category=ResourceWarning
+ )
+
+ def test_compare_trace1(self): # test getLineageTrace() on an intermediate
+ m = sds.full((10, 10), 1)
+ m_res = m + m
+
+ python_trace = [x.strip() for x in m_res.get_lineage_trace().split("\n")]
Review comment:
I guess that depends on what one wants to do with the trace. String is ubiquitous, woks in dml and python. Using other data structures might need different handling. But can be done if that helps the querying interface on top the lineage storage.
----------------------------------------------------------------
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
[GitHub] [systemml] phaniarnab commented on issue #896: [MINOR] Python Lineage Trace Test Change
Posted by GitBox <gi...@apache.org>.
phaniarnab commented on issue #896:
URL: https://github.com/apache/systemml/pull/896#issuecomment-618478532
This looks very good. Thanks for making the test better. As discussed offline, comparing only the internal nodes might solve the problem of metadata mismatch.
----------------------------------------------------------------
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
[GitHub] [systemml] mboehm7 commented on pull request #896: [MINOR] Python Lineage Trace Test Change
Posted by GitBox <gi...@apache.org>.
mboehm7 commented on pull request #896:
URL: https://github.com/apache/systemml/pull/896#issuecomment-619442689
Yes, git is able to detect plain refactorings - the problem usually arises, if a commit does refactoring AND makes major code changes.
----------------------------------------------------------------
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
[GitHub] [systemml] Baunsgaard commented on pull request #896: [MINOR] Python Lineage Trace Test Change
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on pull request #896:
URL: https://github.com/apache/systemml/pull/896#issuecomment-619540385
I will note it down, and try to make it so next time :)
----------------------------------------------------------------
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