Baunsgaard commented on code in PR #1905: URL: https://github.com/apache/systemds/pull/1905#discussion_r1325684493
########## scripts/builtin/img_mirror_linearized.dml: ########## @@ -0,0 +1,82 @@ +#------------------------------------------------------------- Review Comment: You might want to separate the PRs completely to not include mirror inside this PR. ########## scripts/builtin/img_translate_linearized.dml: ########## @@ -0,0 +1,73 @@ +#------------------------------------------------------------- +# +# 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. +# +#------------------------------------------------------------- + +# This function has the same functionality with img_translate but it handles multiple images at +# the same time. Each row of the input and output matrix represents a linearized image/matrix +# It translates the image and Optionally resizes the image (without scaling). +# INPUT: +# ---------------------------------------------------------------------------------------------- +# img_in_linearized Input matrix/image (every row represents a linearized matrix/image) +# offset_x The distance to move the image in x direction +# offset_y The distance to move the image in y direction +# out_w Width of the output image +# out_h Height of the output image +# fill_value the background of the image Review Comment: consistent capital letter ########## scripts/builtin/img_translate_linearized.dml: ########## @@ -0,0 +1,73 @@ +#------------------------------------------------------------- +# +# 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. +# +#------------------------------------------------------------- + +# This function has the same functionality with img_translate but it handles multiple images at +# the same time. Each row of the input and output matrix represents a linearized image/matrix +# It translates the image and Optionally resizes the image (without scaling). +# INPUT: +# ---------------------------------------------------------------------------------------------- +# img_in_linearized Input matrix/image (every row represents a linearized matrix/image) +# offset_x The distance to move the image in x direction +# offset_y The distance to move the image in y direction +# out_w Width of the output image +# out_h Height of the output image +# fill_value the background of the image +# original_w width of the original 2D images +# original_h height of the original 2D images +# -------------------------------------------------------------------------------------------- +# +# OUTPUT: +# -------------------------------------------------------------------------------------------- +# img_out_linearized Output matrix/image (every row represents a linearized matrix/image) +# -------------------------------------------------------------------------------------------- + +m_img_translate_linearized = function(Matrix[Double] img_in_linearized, +Double offset_x, Double offset_y, Integer out_w, Integer out_h, Double fill_value, +Integer original_w, Integer original_h) return (Matrix[Double] img_out_linearized) { + + num_images = nrow(img_in_linearized) + w = original_w + h = original_h + # Round offsets to nearest integer + offset_x = round(offset_x) + offset_y = round(offset_y) + img_out_linearized = matrix(fill_value, rows=num_images, cols=out_w * out_h) + for (i in 1:num_images) { + img_row = img_in_linearized[i,] + + for (y in 1:out_h) { + for (x in 1:out_w) { + src_x = x - offset_x + src_y = y - offset_y + + # Validate indices to be in the valid range + if (src_x >= 1 & src_x <= w & src_y >= 1 & src_y <= h) { + src_index = (src_y - 1) * w + src_x + dest_index = (y - 1) * out_w + x + + if (src_index >= 1 & src_index <= ncol(img_row) & dest_index >= 1 & dest_index <= ncol(img_out_linearized)) { + img_out_linearized[i, dest_index] = img_row[1, src_index] + } + } + } + } + } +} Review Comment: newline in end of file. ########## scripts/builtin/img_translate_linearized.dml: ########## @@ -0,0 +1,73 @@ +#------------------------------------------------------------- +# +# 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. +# +#------------------------------------------------------------- + +# This function has the same functionality with img_translate but it handles multiple images at +# the same time. Each row of the input and output matrix represents a linearized image/matrix +# It translates the image and Optionally resizes the image (without scaling). +# INPUT: +# ---------------------------------------------------------------------------------------------- +# img_in_linearized Input matrix/image (every row represents a linearized matrix/image) +# offset_x The distance to move the image in x direction +# offset_y The distance to move the image in y direction +# out_w Width of the output image +# out_h Height of the output image +# fill_value the background of the image +# original_w width of the original 2D images +# original_h height of the original 2D images +# -------------------------------------------------------------------------------------------- +# +# OUTPUT: +# -------------------------------------------------------------------------------------------- +# img_out_linearized Output matrix/image (every row represents a linearized matrix/image) +# -------------------------------------------------------------------------------------------- + +m_img_translate_linearized = function(Matrix[Double] img_in_linearized, +Double offset_x, Double offset_y, Integer out_w, Integer out_h, Double fill_value, +Integer original_w, Integer original_h) return (Matrix[Double] img_out_linearized) { + + num_images = nrow(img_in_linearized) + w = original_w + h = original_h + # Round offsets to nearest integer + offset_x = round(offset_x) + offset_y = round(offset_y) + img_out_linearized = matrix(fill_value, rows=num_images, cols=out_w * out_h) + for (i in 1:num_images) { + img_row = img_in_linearized[i,] + + for (y in 1:out_h) { + for (x in 1:out_w) { Review Comment: parfor? ########## scripts/builtin/img_translate_linearized.dml: ########## @@ -0,0 +1,73 @@ +#------------------------------------------------------------- +# +# 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. +# +#------------------------------------------------------------- + +# This function has the same functionality with img_translate but it handles multiple images at +# the same time. Each row of the input and output matrix represents a linearized image/matrix +# It translates the image and Optionally resizes the image (without scaling). +# INPUT: +# ---------------------------------------------------------------------------------------------- +# img_in_linearized Input matrix/image (every row represents a linearized matrix/image) +# offset_x The distance to move the image in x direction +# offset_y The distance to move the image in y direction +# out_w Width of the output image +# out_h Height of the output image +# fill_value the background of the image +# original_w width of the original 2D images +# original_h height of the original 2D images +# -------------------------------------------------------------------------------------------- +# +# OUTPUT: +# -------------------------------------------------------------------------------------------- +# img_out_linearized Output matrix/image (every row represents a linearized matrix/image) +# -------------------------------------------------------------------------------------------- + +m_img_translate_linearized = function(Matrix[Double] img_in_linearized, +Double offset_x, Double offset_y, Integer out_w, Integer out_h, Double fill_value, +Integer original_w, Integer original_h) return (Matrix[Double] img_out_linearized) { + + num_images = nrow(img_in_linearized) + w = original_w + h = original_h + # Round offsets to nearest integer + offset_x = round(offset_x) + offset_y = round(offset_y) + img_out_linearized = matrix(fill_value, rows=num_images, cols=out_w * out_h) + for (i in 1:num_images) { + img_row = img_in_linearized[i,] + + for (y in 1:out_h) { + for (x in 1:out_w) { + src_x = x - offset_x + src_y = y - offset_y + + # Validate indices to be in the valid range + if (src_x >= 1 & src_x <= w & src_y >= 1 & src_y <= h) { + src_index = (src_y - 1) * w + src_x + dest_index = (y - 1) * out_w + x + + if (src_index >= 1 & src_index <= ncol(img_row) & dest_index >= 1 & dest_index <= ncol(img_out_linearized)) { + img_out_linearized[i, dest_index] = img_row[1, src_index] Review Comment: see if we can avoid single cell assignment. this makes it very slow. ########## scripts/builtin/img_translate_linearized.dml: ########## @@ -0,0 +1,73 @@ +#------------------------------------------------------------- +# +# 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. +# +#------------------------------------------------------------- + +# This function has the same functionality with img_translate but it handles multiple images at +# the same time. Each row of the input and output matrix represents a linearized image/matrix +# It translates the image and Optionally resizes the image (without scaling). +# INPUT: +# ---------------------------------------------------------------------------------------------- +# img_in_linearized Input matrix/image (every row represents a linearized matrix/image) +# offset_x The distance to move the image in x direction +# offset_y The distance to move the image in y direction +# out_w Width of the output image +# out_h Height of the output image +# fill_value the background of the image +# original_w width of the original 2D images +# original_h height of the original 2D images +# -------------------------------------------------------------------------------------------- +# +# OUTPUT: +# -------------------------------------------------------------------------------------------- +# img_out_linearized Output matrix/image (every row represents a linearized matrix/image) +# -------------------------------------------------------------------------------------------- + +m_img_translate_linearized = function(Matrix[Double] img_in_linearized, +Double offset_x, Double offset_y, Integer out_w, Integer out_h, Double fill_value, +Integer original_w, Integer original_h) return (Matrix[Double] img_out_linearized) { Review Comment: indent here with 4 space, to clearly indicate the function definition the rest of the file is indented with 2. -- 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-unsubscr...@systemds.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
