[GitHub] [systemml] asfgit closed pull request #911: [SYSTEMDS-344]Unmark functions/SBs containing non-determinism

[GitBox]

asfgit closed pull request #911:
URL: https://github.com/apache/systemml/pull/911


   



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:
us...@infra.apache.org

[GitHub] [systemml] mboehm7 commented on pull request #911: [SYSTEMDS-344]Unmark functions/SBs containing non-determinism

[GitBox]

mboehm7 commented on pull request #911:
URL: https://github.com/apache/systemml/pull/911#issuecomment-629858562


   LGTM - thanks @phaniarnab. The only missing piece was to copy this 
information during parfor worker creation (where we deep copy functions and 
statement blocks to allow to independent, and thus uncontended recompilation) - 
dropping it would have potentially created incorrect results. Other than that 
and a few unnecessary imports this is great! 



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:
us...@infra.apache.org

[GitHub] [systemml] Baunsgaard commented on pull request #910: [MINOR] L2SVM, now directly support 1 Hot input

[GitBox]

Baunsgaard commented on pull request #910:
URL: https://github.com/apache/systemml/pull/910#issuecomment-629857227


   Thanks for the Review @mboehm7 , I will make the modifications. You are a 
bit early, because i decided to change many things again again, i should 
perhaps have marked the PR as WIP again :confused: 



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:
us...@infra.apache.org

[GitHub] [systemml] mboehm7 commented on a change in pull request #910: [MINOR] L2SVM, now directly support 1 Hot input

[GitBox]

mboehm7 commented on a change in pull request #910:
URL: https://github.com/apache/systemml/pull/910#discussion_r426299182



##
File path: scripts/builtin/l2svm.dml
##
@@ -44,58 +47,62 @@
 
 
 m_l2svm = function(Matrix[Double] X, Matrix[Double] Y, Boolean intercept = 
FALSE,
-Double epsilon = 0.001, Double lambda = 1, Integer maxiterations = 100, 
Boolean verbose = FALSE)
+  Double epsilon = 0.001, Double lambda = 1, Integer 
max_iterations = 100, 
+  Boolean verbose = FALSE, Integer column_id = -1)

Review comment:
   This Python-like indentation is a bit unpleasant to the eye.

##
File path: scripts/builtin/confusionMatrix.dml
##
@@ -0,0 +1,51 @@
+#-
+#
+# 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.
+#
+#-
+
+# INPUT PARAMETERS:
+# 
-
+# NAMETYPEDEFAULT MEANING
+# 
-
+# P   Double  --- matrix of Predictions
+# Y   Double  --- matrix of Golden standard One Hot Encoded
+# 
-
+# OUTPUT:
+# 
-
+# NAMETYPEDEFAULT MEANING
+# 
-
+# ConfusionMatrix Double  --- The Confusion Matrix
+
+
+m_confusionMatrix = function(Matrix[Double] P, Matrix[Double] Y)
+  return(Matrix[Double] confusion)
+{
+  if(nrow(P) != nrow(Y))
+stop("CONFUSION MATRIX: The number of rows have to be equal in both P 
["+nrow(P)+"] and Y ["+nrow(Y)+"]")
+
+  res = matrix(0, max(P), max(Y))
+  for(i in 1:nrow(P) ){
+row = as.scalar(Y[i,1])
+col = as.scalar(P[i,1])
+
+res[row,col] = as.scalar(res[row,col]) +  1
+  }

Review comment:
   Please replace the entire for loop with a `table(P[1,], Y[1,])` (it's 
fine to have a remapping to table). Also note that the construction of `res` 
use inconsistent lhs/rhs matrices (P/Y vs Y/P).

##
File path: scripts/builtin/l2svm.dml
##
@@ -112,32 +119,37 @@ Double epsilon = 0.001, Double lambda = 1, Integer 
maxiterations = 100, Boolean
   step_sz = step_sz - g/h
   continue1 = (g*g/h >= epsilon)
 }
-  
+
 #update weights
 w = w + step_sz*s
 Xw = Xw + step_sz*Xd
-  
+
 out = 1 - Y * Xw
 sv = (out > 0)
 out = sv * out
 obj = 0.5 * sum(out * out) + lambda/2 * sum(w * w)
 g_new = t(X) %*% (out * Y) - lambda * w
-  
-
-if(verbose) print("Iter, Obj "+ iter + ", "+obj)
-  
+
+if(verbose) {
+  if(column_id != -1){
+print("Iter:" + toString(iter) + ", Col:" + column_id + ", Obj:" + obj)
+  } 
+  else {
+print("Iter:" + toString(iter) + ", Obj:" + obj)
+  }
+} 
+
 tmp = sum(s * g_old)
 if(step_sz*tmp < epsilon*obj){
   continue = 0
 }

Review comment:
   Somehow I missed this on the initial commit, but this can become: 
`continue = (step_sz*tmp >= epsilon*obj & sum(s^2) != 0);`

##
File path: scripts/builtin/l2svm.dml
##
@@ -112,32 +119,37 @@ Double epsilon = 0.001, Double lambda = 1, Integer 
maxiterations = 100, Boolean
   step_sz = step_sz - g/h
   continue1 = (g*g/h >= epsilon)
 }
-  
+
 #update weights
 w = w + step_sz*s
 Xw = Xw + step_sz*Xd
-  
+
 out = 1 - Y * Xw
 sv = (out > 0)
 out = sv * out
 obj = 0.5 * sum(out * out) + lambda/2 * sum(w * w)
 g_new = t(X) %*% (out * Y) - lambda * w
-  
-
-if(verbose) print("Iter, Obj "+ iter + ", "+obj)
-  
+
+if(verbose) {
+  if(column_id != -1){
+print("Iter:" + toString(iter) + ", Col:" + column_id + ", Obj:" + obj)
+  } 
+  else {
+print("Iter:" + toString(iter) + ", Obj:" + obj)
+  }

Review comment:
   could we maybe replace this with something more 

Re: Docker Container Organisation

[Matthias Boehm]

thanks for following-up on this. @Berthold: do I remember correctly that 
you looked into a similar setup a while ago?


Regards,
Matthias

On 5/15/2020 7:46 PM, Baunsgaard, Sebastian wrote:

Hi SystemDS developers.


Currently we have some docker containers that are associated with my  personal 
docker user.

It has been brought to my attention that this is not appropriate, and I would 
therefore suggest we make a Docker organization.

But this rises some questions:


- Is there a normal procedure for Apache Projects for docker containers?

- If not, then furthermore what should we call this docker organization?

- and how do we/I distribute rights to maintain this docker organization to the 
committers?


For notice of how important the docker container is, it has currently been 
downloaded and installed over 10k times for testing our system, and for each 
instance it has saved 1 min of testing time on GitHub Workflows (1 week total 
time so far).


Furthermore we need to find a solution hopefully soon so that we can set 
automatic updates of our docker image up, since when updating dependencies at 
the moment, require me to manually update the docker image. This is for 
instance required for a current PR (https://github.com/apache/systemml/pull/914)


Searching a little on for Apache docker images of different projects it looks like 
other persons have repositories that contain images for specific systems. Like 
Microsoft having a Spark image 
(https://hub.docker.com/_/microsoft-mmlspark-release).


best regards

Sebastian