Baunsgaard commented on code in PR #1983: URL: https://github.com/apache/systemds/pull/1983#discussion_r1492377862
########## src/main/java/org/apache/sysds/runtime/instructions/cp/MultiReturnComplexMatrixBuiltinCPInstruction.java: ########## @@ -0,0 +1,167 @@ +/* + * 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. + */ + +package org.apache.sysds.runtime.instructions.cp; + +import java.util.ArrayList; +import java.util.List; + +import org.apache.commons.lang3.tuple.Pair; +import org.apache.sysds.common.Types.DataType; +import org.apache.sysds.common.Types.ValueType; +import org.apache.sysds.runtime.DMLRuntimeException; +import org.apache.sysds.runtime.controlprogram.context.ExecutionContext; +import org.apache.sysds.runtime.instructions.InstructionUtils; +import org.apache.sysds.runtime.lineage.LineageItem; +import org.apache.sysds.runtime.lineage.LineageItemUtils; +import org.apache.sysds.runtime.matrix.data.LibCommonsMath; +import org.apache.sysds.runtime.matrix.data.MatrixBlock; +import org.apache.sysds.runtime.matrix.operators.Operator; + +public class MultiReturnComplexMatrixBuiltinCPInstruction extends ComputationCPInstruction { + + protected ArrayList<CPOperand> _outputs; + + private MultiReturnComplexMatrixBuiltinCPInstruction(Operator op, CPOperand input1, CPOperand input2, + ArrayList<CPOperand> outputs, String opcode, + String istr) { + super(CPType.MultiReturnBuiltin, op, input1, input2, outputs.get(0), opcode, istr); + _outputs = outputs; + } + + private MultiReturnComplexMatrixBuiltinCPInstruction(Operator op, CPOperand input1, ArrayList<CPOperand> outputs, + String opcode, + String istr) { + super(CPType.MultiReturnBuiltin, op, input1, null, outputs.get(0), opcode, istr); + _outputs = outputs; + } + + public CPOperand getOutput(int i) { + return _outputs.get(i); + } + + public List<CPOperand> getOutputs() { + return _outputs; + } + + public String[] getOutputNames() { + return _outputs.parallelStream().map(output -> output.getName()).toArray(String[]::new); + } + + public static MultiReturnComplexMatrixBuiltinCPInstruction parseInstruction(String str) { + String[] parts = InstructionUtils.getInstructionPartsWithValueType(str); + ArrayList<CPOperand> outputs = new ArrayList<>(); + // first part is always the opcode + String opcode = parts[0]; + + if (parts.length == 5 && opcode.equalsIgnoreCase("ifft")) { + // one input and two outputs + CPOperand in1 = new CPOperand(parts[1]); + CPOperand in2 = new CPOperand(parts[2]); + outputs.add(new CPOperand(parts[3], ValueType.FP64, DataType.MATRIX)); + outputs.add(new CPOperand(parts[4], ValueType.FP64, DataType.MATRIX)); + + return new MultiReturnComplexMatrixBuiltinCPInstruction(null, in1, in2, outputs, opcode, str); + } else if (parts.length == 4 && opcode.equalsIgnoreCase("ifft")) { + // one input and two outputs + CPOperand in1 = new CPOperand(parts[1]); + outputs.add(new CPOperand(parts[2], ValueType.FP64, DataType.MATRIX)); + outputs.add(new CPOperand(parts[3], ValueType.FP64, DataType.MATRIX)); + + return new MultiReturnComplexMatrixBuiltinCPInstruction(null, in1, outputs, opcode, str); + } else if (parts.length == 5 && opcode.equalsIgnoreCase("ifft_linearized")) { + // one input and two outputs + CPOperand in1 = new CPOperand(parts[1]); + CPOperand in2 = new CPOperand(parts[2]); + outputs.add(new CPOperand(parts[3], ValueType.FP64, DataType.MATRIX)); + outputs.add(new CPOperand(parts[4], ValueType.FP64, DataType.MATRIX)); + + return new MultiReturnComplexMatrixBuiltinCPInstruction(null, in1, in2, outputs, opcode, str); + } else if (parts.length == 4 && opcode.equalsIgnoreCase("ifft_linearized")) { + // one input and two outputs + CPOperand in1 = new CPOperand(parts[1]); + outputs.add(new CPOperand(parts[2], ValueType.FP64, DataType.MATRIX)); + outputs.add(new CPOperand(parts[3], ValueType.FP64, DataType.MATRIX)); + + return new MultiReturnComplexMatrixBuiltinCPInstruction(null, in1, outputs, opcode, str); + } + + { Review Comment: this bracket does not make sense ########## src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixFourier.java: ########## @@ -0,0 +1,309 @@ +/* + * 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. + */ + +package org.apache.sysds.runtime.matrix.data; + +import org.apache.commons.math3.util.FastMath; + +public class LibMatrixFourier { + + /** + * Function to perform FFT for two given matrices. The first one represents the real values and the second one the + * imaginary values. The output also contains one matrix for the real and one for the imaginary values. + * + * @param re matrix object representing the real values + * @param im matrix object representing the imaginary values + * @return array of two matrix blocks + */ + public static MatrixBlock[] fft(MatrixBlock re, MatrixBlock im) { + + int rows = re.getNumRows(); + int cols = re.getNumColumns(); + if(!isPowerOfTwo(rows) || !isPowerOfTwo(cols)) + throw new RuntimeException("false dimensions"); + + fft(re.getDenseBlockValues(), im.getDenseBlockValues(), rows, cols, true); + + return new MatrixBlock[] {re, im}; + } + + /** + * Function to perform IFFT for two given matrices. The first one represents the real values and the second one the + * imaginary values. The output also contains one matrix for the real and one for the imaginary values. + * + * @param re matrix object representing the real values + * @param im matrix object representing the imaginary values + * @return array of two matrix blocks + */ + public static MatrixBlock[] ifft(MatrixBlock re, MatrixBlock im) { + + int rows = re.getNumRows(); + int cols = re.getNumColumns(); + if(!isPowerOfTwo(rows) || !isPowerOfTwo(cols)) + throw new RuntimeException("false dimensions"); + + ifft(re.getDenseBlockValues(), im.getDenseBlockValues(), rows, cols, true); + + return new MatrixBlock[] {re, im}; + } + + /** + * Function to perform FFT for each row of two given matrices. The first one represents the real values and the + * second one the imaginary values. The output also contains one matrix for the real and one for the imaginary + * values. + * + * @param re matrix object representing the real values + * @param im matrix object representing the imaginary values + * @return array of two matrix blocks + */ + public static MatrixBlock[] fft_linearized(MatrixBlock re, MatrixBlock im) { + + int rows = re.getNumRows(); + int cols = re.getNumColumns(); + if(!isPowerOfTwo(cols)) + throw new RuntimeException("false dimensions"); + + fft(re.getDenseBlockValues(), im.getDenseBlockValues(), rows, cols, false); + + return new MatrixBlock[] {re, im}; + } + + /** + * Function to perform IFFT for each row of two given matrices. The first one represents the real values and the + * second one the imaginary values. The output also contains one matrix for the real and one for the imaginary + * values. + * + * @param re matrix object representing the real values + * @param im matrix object representing the imaginary values + * @return array of two matrix blocks + */ + public static MatrixBlock[] ifft_linearized(MatrixBlock re, MatrixBlock im) { + + int rows = re.getNumRows(); + int cols = re.getNumColumns(); + if(!isPowerOfTwo(cols)) + throw new RuntimeException("false dimensions"); + + ifft(re.getDenseBlockValues(), im.getDenseBlockValues(), rows, cols, false); + + return new MatrixBlock[] {re, im}; + } + + /** + * Function to perform FFT for two given double arrays. The first one represents the real values and the second one + * the imaginary values. Both arrays get updated and contain the result. + * + * @param re array representing the real values + * @param im array representing the imaginary values + * @param rows number of rows + * @param cols number of rows + * @param inclColCalc if true, fft is also calculated for each column, otherwise only for each row + */ + public static void fft(double[] re, double[] im, int rows, int cols, boolean inclColCalc) { + + double[] re_inter = new double[rows * cols]; + double[] im_inter = new double[rows * cols]; + + for(int i = 0; i < rows; i++) { + fft_one_dim(re, im, re_inter, im_inter, i * cols, (i + 1) * cols, cols, 1); + } + + if(inclColCalc) { + for(int j = 0; j < cols; j++) { + fft_one_dim(re, im, re_inter, im_inter, j, j + rows * cols, rows, cols); + } + } + + } + + /** + * Function to perform IFFT for two given double arrays. The first one represents the real values and the second one + * the imaginary values. Both arrays get updated and contain the result. + * + * @param re array representing the real values + * @param im array representing the imaginary values + * @param rows number of rows + * @param cols number of rows + * @param inclColCalc if true, fft is also calculated for each column, otherwise only for each row + */ + public static void ifft(double[] re, double[] im, int rows, int cols, boolean inclColCalc) { + + double[] re_inter = new double[rows * cols]; + double[] im_inter = new double[rows * cols]; + + if(inclColCalc) { + for(int j = 0; j < cols; j++) { + ifft_one_dim(re, im, re_inter, im_inter, j, j + rows * cols, rows, cols); + } + } + + for(int i = 0; i < rows; i++) { + ifft_one_dim(re, im, re_inter, im_inter, i * cols, (i + 1) * cols, cols, 1); Review Comment: consider if this call can be parallel: The code would be something along the lines of: ``` ExecutionContext pool = CommonThreadPool.get(k) for( ...) { pool.submit( () -> ifft_one_dim(re,im,re_inter, im_inter ...) } ``` k is number of threads, and you can get that argument from CommonMath file. Thanks ########## src/main/java/org/apache/sysds/runtime/matrix/data/LibCommonsMath.java: ########## @@ -252,6 +295,104 @@ private static EigenDecomposition computeEigenRegularized(MatrixBlock in) { DataConverter.convertToArray2DRowRealMatrix(in2)); } + /** + * Function to perform FFT on a given matrix. + * + * @param re matrix object + * @return array of matrix blocks + */ + private static MatrixBlock[] computeFFT(MatrixBlock re) { + if(re == null) + throw new DMLRuntimeException("Invalid empty block"); + if (re.isEmptyBlock(false)) { + // Return the original matrix as the result + return new MatrixBlock[]{re, new MatrixBlock(re.getNumRows(), re.getNumColumns(), true)}; // Assuming you need to return two matrices: the real part and an imaginary part initialized to 0. + } + // run fft + re.sparseToDense(); + return fft(re); + } + + private static boolean isMatrixAllZeros(MatrixBlock matrix) { Review Comment: This is not a good check, please use isEmpty() and nonzeros count to verify if there is values in the matrix. ########## src/main/java/org/apache/sysds/runtime/matrix/data/LibCommonsMath.java: ########## @@ -113,9 +134,31 @@ else if (opcode.equals("eigen_qr")) return computeEigenQR(in, threads); else if (opcode.equals("svd")) return computeSvd(in); + else if (opcode.equals("fft")) + return computeFFT(in); + else if (opcode.equals("ifft")) + return computeIFFT(in, null); Review Comment: you can overload the computeIFFT function to remove this null argument. -- 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
