You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@systemds.apache.org by "anishsapkota (via GitHub)" <gi...@apache.org> on 2023/09/11 12:35:15 UTC
[GitHub] [systemds] anishsapkota opened a new pull request, #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
anishsapkota opened a new pull request, #1843:
URL: https://github.com/apache/systemds/pull/1843
This is where i intend to add the native bindings for intel MKL image processing.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on a diff in pull request #1843: [SYSTEMDS-3546] good start
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on code in PR #1843:
URL: https://github.com/apache/systemds/pull/1843#discussion_r1231639035
##########
src/main/cpp/libAnish.cpp:
##########
Review Comment:
Hi Sebastian,
i have successfully implemented a mkl degmm for test purpose . For that i did the native function implementation in systemds.cpp, function defination in systemds.h and created a native interface in NativeHelper.java. In src/test/java/org/apache/sysds/test/functions i have also implemented a main function in Java to test the functionality. it works with mkl and succefully calls the native cpp function.
However, i had to comment out the conflicting functions of openBLAS in order to build the shared lib. :(
i will need your help to find out another way around.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota closed pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota closed pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
URL: https://github.com/apache/systemds/pull/1843
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1712542829
Hi @Baunsgaard ,
I have been loading the shared libraries manually on my tests as the Native Helper currently supports LINUX and Windows. I have been working using MAC_OS and the libraries are therefore not loaded automatically. Could you please suggest me something on this?
Apart from this, I have created a new file systemds_img.cpp and add the methods that I have implemented there, added nativ function tests to JavaTest.yaml and created a new Java Class ImgNativeHelper that extends NativeHelper.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1722585242
**image_rotate**
SystemDS Statistics:
Total elapsed time: 1,842 sec.
Total compilation time: 0,042 sec.
**Total execution time: 1,800 sec.**
Number of compiled Spark inst: 1.
Number of executed Spark inst: 0.
Cache hits (Mem/Li/WB/FS/HDFS): 1835090/0/0/0/1.
Cache writes (Li/WB/FS/HDFS): 3/14/0/1.
Cache times (ACQr/m, RLS, EXP): 0,946/0,352/1,060/0,058 sec.
HOP DAGs recompiled (PRED, SB): 0/0.
HOP DAGs recompile time: 0,167 sec.
Spark ctx create time (lazy): 0,000 sec.
Spark trans counts (par,bc,col):0/0/0.
Spark trans times (par,bc,col): 0,000/0,000/0,000 secs.
Spark async. count (pf,bc,op): 0/0/0.
ParFor loops optimized: 1.
ParFor optimize time: 0,001 sec.
ParFor initialize time: 0,004 sec.
ParFor result merge time: 0,028 sec.
ParFor total update in-place: 0/262144/262168
Total JIT compile time: 36.02 sec.
Total JVM GC count: 175.
Total JVM GC time: 0.374 sec.
Heavy hitter instructions:
# Instruction Time(s) Count
1 rightIndex 3,288 786443
**2 m_img_rotate 1,711 1**
**3 m_img_transform 1,711 1**
4 rmvar 1,434 2359350
5 leftIndex 1,213 262168
6 createvar 0,997 1048631
7 castdts 0,846 524294
8 && 0,544 786432
9 mvvar 0,392 524300
10 < 0,355 524288
**image_cutout**
SystemDS Statistics:
Total elapsed time: 0,108 sec.
Total compilation time: 0,042 sec.
**Total execution time: 0,065 sec.**
Number of compiled Spark inst: 1.
Number of executed Spark inst: 0.
Cache hits (Mem/Li/WB/FS/HDFS): 2/0/0/0/1.
Cache writes (Li/WB/FS/HDFS): 0/1/0/1.
Cache times (ACQr/m, RLS, EXP): 0,032/0,000/0,000/0,033 sec.
HOP DAGs recompiled (PRED, SB): 0/0.
HOP DAGs recompile time: 0,000 sec.
Spark ctx create time (lazy): 0,000 sec.
Spark trans counts (par,bc,col):0/0/0.
Spark trans times (par,bc,col): 0,000/0,000/0,000 secs.
Spark async. count (pf,bc,op): 0/0/0.
Total JIT compile time: 13.577 sec.
Total JVM GC count: 176.
Total JVM GC time: 0.368 sec.
Heavy hitter instructions:
# Instruction Time(s) Count
1 write 0,033 1
**2 sp_rblk 0,032 1**
3 rmvar 0,001 2
4 leftIndex 0,000 1
5 rand 0,000 1
6 createvar 0,000 4
7 + 0,000 2
8 < 0,000 1
9 mvvar 0,000 8
10 - 0,000 2
**image_crop**
New w/h=409.0/409.0
SystemDS Statistics:
Total elapsed time: 0,134 sec.
Total compilation time: 0,029 sec.
**Total execution time: 0,105 sec.**
Number of compiled Spark inst: 1.
Number of executed Spark inst: 0.
Cache hits (Mem/Li/WB/FS/HDFS): 9/0/0/0/1.
Cache writes (Li/WB/FS/HDFS): 1/8/0/1.
Cache times (ACQr/m, RLS, EXP): 0,037/0,000/0,000/0,063 sec.
HOP DAGs recompiled (PRED, SB): 0/0.
HOP DAGs recompile time: 0,000 sec.
Spark ctx create time (lazy): 0,000 sec.
Spark trans counts (par,bc,col):0/0/0.
Spark trans times (par,bc,col): 0,000/0,000/0,000 secs.
Spark async. count (pf,bc,op): 0/0/0.
Total JIT compile time: 36.052 sec.
Total JVM GC count: 176.
Total JVM GC time: 0.367 sec.
Heavy hitter instructions:
# Instruction Time(s) Count
1 write 0,063 1
2 sp_rblk 0,037 1
**3 m_img_crop 0,005 1**
4 - 0,002 4
5 rmempty 0,001 1
6 rshape 0,000 3
7 rmvar 0,000 8
8 + 0,000 8
9 leftIndex 0,000 1
10 rand 0,000 2
**image_translate**
SystemDS Statistics:
Total elapsed time: 0,166 sec.
Total compilation time: 0,049 sec.
**Total execution time: 0,117 sec.**
Number of compiled Spark inst: 1.
Number of executed Spark inst: 0.
Cache hits (Mem/Li/WB/FS/HDFS): 3/0/0/0/1.
Cache writes (Li/WB/FS/HDFS): 0/3/0/1.
Cache times (ACQr/m, RLS, EXP): 0,039/0,000/0,000/0,077 sec.
HOP DAGs recompiled (PRED, SB): 0/0.
HOP DAGs recompile time: 0,000 sec.
Spark ctx create time (lazy): 0,000 sec.
Spark trans counts (par,bc,col):0/0/0.
Spark trans times (par,bc,col): 0,000/0,000/0,000 secs.
Spark async. count (pf,bc,op): 0/0/0.
Total JIT compile time: 13.584 sec.
Total JVM GC count: 176.
Total JVM GC time: 0.367 sec.
Heavy hitter instructions:
# Instruction Time(s) Count
1 write 0,077 1
2 sp_rblk 0,039 1
**3 m_img_translate 0,001 1**
4 leftIndex 0,000 1
5 rmvar 0,000 12
6 rand 0,000 1
7 rightIndex 0,000 1
8 createvar 0,000 5
9 < 0,000 8
10 mvvar 0,000 10
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SYSTEMDS-3546] Push down image pre-processing to blas/mkl [systemds]
Posted by "corepointer (via GitHub)" <gi...@apache.org>.
corepointer commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1855691217
Hi!
Sorry that I'm a bit late to the discussion.
Our current Intel MKL support ends at version 2019.5 (you noticed the problems with dnn ops not compiling). I would be surprised if all magically worked by just using OneMKL and OneDNN.
We should probably put the native ops testing into a separate test suite and use a Docker container that supports it.
Furthermore, moving to OneAPI would change external behavior and require a major version bump.
Regards, Mark
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1719098183
> i am unfortunately unaware what needs to be loaded when, there must be some sequence or elements missing. this libmkl_rt.so is what is says is missing now, not your build mkl file.
I was misunderstanding the log output, I have added LD_LIBRARY_PATH and additional ls statements to see the outputs, could you please lt it run once again, @Baunsgaard
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1722190295
> > My main comment is the comparison is not fair. There are some things we can do to improve it if you want (read the detailed comments for more).
>
> Hi @Baunsgaard , thank your for reviewing the code and the suggestions. Rest assured, I will address all of them after my presentation.
>
> could you be more specific, how can I make the comparison fairer? I will be incorporating your previous suggestions of calculating std and analysing dml execution time using -stats flag. Do you have any further suggestions? It would be interesting for the presentation to address this issue before that.
The fairest comparison would be to integrate the native instruction as a DML supported command. It does not take long to make. But requires you to know where to do it.
Once integrated we can compare with the overhead of the system on both executions and thereby get a fair comparison.
For now the stats comparison is close enough.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1713779956
> Hi @Baunsgaard , I am waiting for the tests to run, so that we can merge.
>
> regarding the performance results, should I be comparing the performance between OpenBlas and Intel MKL ? or with the existing DML implementation?
You need to compare the results between all possible ones you have.
But most importantly between DML Java version and your MKL.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1713929461
> java.lang.UnsatisfiedLinkError: /github/workspace/src/main/cpp/lib/libsystemds_openblas-Linux-x86_64.so: libopenblas.so.0: cannot open shared object file: No such file or directory
>
> I still can't figure it out why it can't find the lib. could you please review the static block from my last commit ?
it does not look wrong in the last 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.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on a diff in pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on code in PR #1843:
URL: https://github.com/apache/systemds/pull/1843#discussion_r1320670893
##########
src/test/java/org/apache/sysds/test/functions/nativ/ImgUtilsTest.java:
##########
@@ -0,0 +1,412 @@
+/*
+ * 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.test.functions.nativ;
+
+import org.apache.sysds.utils.ImgNativeHelper;
+import org.apache.sysds.utils.NativeHelper;
+import org.junit.Test;
+
+import static org.junit.Assert.assertArrayEquals;
+
+public class ImgUtilsTest {
+
+ /*static {
+ System.loadLibrary("systemds_openblas-Darwin-x86_64");
Review Comment:
No we compile it on each test run for that machine. You can find the appropriate code in our GitHub action script.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1625182480
Hi @anishsapkota,
How is it going? is there uncommitted progress or do you need any further comments?
Also, feel free if any of the comments are resolved, to click resolved yourself.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1714347436
Performance Evaluation averaged over 100 runs:
<img width="518" alt="Screenshot 2023-09-11 at 19 59 44" src="https://github.com/apache/systemds/assets/68165368/797db0d7-1c44-4475-8d19-94efa751b152">
I observed that the first run takes significantly more time in comparison to later runs, although the input matrices are completely randomised.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1718879255
Hi @j143 ,
At the moment https://github.com/apache/systemds/pull/1843/commits/779131a24d26d8f210fc77340284a39d85deaae7, I am really confused that the shared object libsystemds_mkl-Linux-x86_64.so is not being found under
/github/workspace/src/main/cpp/lib although the other shared object libsystemds_openblas-Linux-x86_64.so is being loaded from same path and the tests run through.
Moreover random tests are failing, like countDistinct, built-in.part2, which would otherwise pass, if I were not to install intel oneAPi.
The biggest problem is the header file "mkl_dnn.h" which is imported into libmatrixdnn.cpp, it doesn't exist in Intel oneApi include folder.
https://github.com/apache/systemds/blob/main/src/main/cpp/libmatrixdnn.cpp
https://github.com/01org/mkl-dnn/releases/download/v0.10/mklml_lnx_2018.0.20170908.tgz
This release contains the "mkl_dnn.h"
should I try to unzip it and add it somewhere and export its path to CPATH ?
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1715452395
Hi @Baunsgaard
The build was successful for Linux using openblas, little progress.
However I noticed that the built shared library location is different from the workspace.
/home/runner/work/systemds/systemds/src/main/cpp/lib/libsystemds_openblas-Linux-x86_64.so
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1714503994
Looking at the other PR, i remembered,
Please also test with uneven images with different number of row and column pixels.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SYSTEMDS-3546] Push down image pre-processing to blas/mkl [systemds]
Posted by "j143 (via GitHub)" <gi...@apache.org>.
j143 commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1794051051
Hi @anishsapkota -- thanks for your contribution. do you need some help here?
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1720975168
> @Baunsgaard @j143 Hi, after some intensive trying using **gitpod** I could conclude following things:
>
> 1. **libmkl_rt.so cannot open the file: no such file or directory** this error was because ldconfig could not find the exported LD_LIBRARY_PATH. I could solve this problem by adding a **.conf file** with **/opt/intel/oneapi/mkl/latest/lib/intel64** in **/etc/ld.so.conf.d** directory and reloading **ldconfig.**
>
> 2. Next Thanks to @j143 I stumbled across his video about Intel MKL installation on ubuntu. If I would do it just like he mentioned using a **offline package installer** which has CLI based UI and requires user interaction ( _I could not find a way to do this using shell script_), everything would work fine after doing what I mentioned in 1.
> If we were to use **apt package installer** instead of offline one, somehow the include folder containing all the necessary header file like "mkl.h" is missing in mkl root folder. I tried using "locate mkl.h" no result.
> I could not find the solution to this.
> Next option would be to install whole intel-basekit which contains MKL as well. It is unfortunately too large ~13 GB I think and could have led to the not enough space error.
>
> 3. Regarding "mkl_dnn.h" .I have mentioned here [[SYSTEMDS-3546] Push down image pre-processing to blas/mkl #1843 (comment)](https://github.com/apache/systemds/pull/1843#issuecomment-1718879255), if we download it unzip it, we have 2 folders in it include and lib, then copy "mkl_dnn.h" and "mkl_dnn_types.h"(second name is something similar) for its include folder to /opt/intel/oneapi/mkl/latest/include (if present) and the libs to /opt/intel/oneapi/mkl/latest/lib/intel64. Using this trick I could compile the cpp files with "mkl_dnn.h"header file as well. **But not recommended it was just an experiment.**
>
> 4. Nevertheless, I could run the benchmarks using both mkl and openblas, also DML implementations using -stats flag in gitpod. Although I could not install mkl properly in GitHub test docker container. I will process the results and post it.
>
>
> @Baunsgaard should I comment out "mkl test case" so that we can merge? or how should I proceed now?
lets go with the Blas test then and add some TODO that reference this PR about the MKL tests, then I will try to make it work at a later time, since i need it for other things.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] j143 commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "j143 (via GitHub)" <gi...@apache.org>.
j143 commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1719009368
> I am really confused that the shared object libsystemds_mkl-Linux-x86_64.so is not being found under
/github/workspace/src/main/cpp/lib although the other shared object libsystemds_openblas-Linux-x86_64.so is being loaded from same path and the tests run through.
you check the github actions workflow logs to find out why this is the case.
> Moreover random tests are failing, like countDistinct, built-in.part2, which would otherwise pass, if I were not to install intel oneAPi.
this can be taken once the above is resolved, I guess.
> should I try to unzip it and add it somewhere and export its path to CPATH ?
The dnn library seems to be published differently. You could choose to download and add it. (we can't commit mkl_dnn.h to the systemds code btw.)
I am running this workflow once again, so that you could get the logs to debug.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1719285147
@Baunsgaard @j143 , could you please let the last commit run?
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on a diff in pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on code in PR #1843:
URL: https://github.com/apache/systemds/pull/1843#discussion_r1327427076
##########
.github/workflows/javaTests.yml:
##########
@@ -81,39 +82,39 @@ jobs:
]
name: ${{ matrix.tests }}
steps:
- - name: Checkout Repository
- uses: actions/checkout@v4
-
-
- - name: ${{ matrix.tests }}
- uses: ./.github/action/
- id: test
- with:
- test-to-run: ${{ matrix.tests }}
-
- - name: Clean Github Artifact Name of Asterisks
- run: |
- ARTIFACT_NAME="transient_jacoco"
- ARTIFACT_NAME+="-${{ matrix.os }}"
- ARTIFACT_NAME+="-java-${{ matrix.java }}"
- ARTIFACT_NAME+="-${{ matrix.javadist }}"
- ARTIFACT_NAME+="-${{ matrix.tests }}"
- ARTIFACT_NAME=${ARTIFACT_NAME//\*/x} # replace * with x
- echo "ARTIFACT_NAME=$ARTIFACT_NAME" >> $GITHUB_ENV
-
- - name: Save Java Test Coverage as Artifact
- uses: actions/upload-artifact@v3
- with:
- name: ${{ env.ARTIFACT_NAME }}
- path: target/jacoco.exec
- retention-days: 1
+ - name: Checkout Repository
+ uses: actions/checkout@v4
Review Comment:
we need to revert here, since the change is only syntax.
##########
src/main/cpp/imgUtils.cpp:
##########
@@ -0,0 +1,243 @@
+/*
+ * 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.
+ */
+
+#include <iostream>
+#include <cmath>
+#include <cstring>
+#include "common.h"
+
+using namespace std;
+
+void printImage(const double* image, int rows, int cols) {
+ for (int i = 0; i < rows; i++) {
+ for (int j = 0; j < cols; j++) {
+ cout << image[i * cols + j] << " ";
+ }
+ cout << endl;
+ }
+ cout << "\n"<< endl;
+}
+
+void img_transform(const double* img_in, int orig_w, int orig_h, int out_w, int out_h, double a, double b, double c, double d,
+ double e, double f, double fill_value, double* img_out) {
+ double divisor = a * e - b * d;
+ if (divisor == 0) {
+ std::cout << "Inverse matrix does not exist! Returning input." << std::endl;
+ for (int i = 0; i < orig_h; i++) {
+ for (int j = 0; j < orig_w; j++) {
+ img_out[i * orig_w + j] = img_in[i * orig_w + j];
+ }
+ }
+ } else {
+ // Create the inverted transformation matrix
+ double T_inv[9];
+ T_inv[0] = e / divisor;
+ T_inv[1] = -b / divisor;
+ T_inv[2] = (b * f - c * e) / divisor;
+ T_inv[3] = -d / divisor;
+ T_inv[4] = a / divisor;
+ T_inv[5] = (c * d - a * f) / divisor;
+ T_inv[6] = 0.0;
+ T_inv[7] = 0.0;
+ T_inv[8] = 1.0;
+
+ // Create the coordinates of output pixel centers linearized in row-major order
+ double* coords = new double[2 * out_w * out_h];
+ for (int i = 0; i < out_h; i++) {
+ for (int j = 0; j < out_w; j++) {
+ coords[2 * (i * out_w + j)] = j + 0.5;
+ coords[2 * (i * out_w + j) + 1] = i + 0.5;
+ }
+ }
+
+ // Perform matrix multiplication to compute sampling pixel indices
+ double* transformed_coords = new double[2 * out_w * out_h];
+ for (int i = 0; i < out_w * out_h; i++) {
+ double x = coords[2 * i];
+ double y = coords[2 * i + 1];
+ transformed_coords[2 * i] = std::floor(T_inv[0] * x + T_inv[1] * y + T_inv[2]) + 1;
+ transformed_coords[2 * i + 1] = std::floor(T_inv[3] * x + T_inv[4] * y + T_inv[5]) + 1;
+ }
+
+ // Fill output image
+ for (int i = 0; i < out_h; i++) {
+ for (int j = 0; j < out_w; j++) {
+ int inx = static_cast<int>(transformed_coords[2 * (i * out_w + j)]) - 1;
+ int iny = static_cast<int>(transformed_coords[2 * (i * out_w + j) + 1]) - 1;
+ if (inx >= 0 && inx < orig_w && iny >= 0 && iny < orig_h) {
+ img_out[i * out_w + j] = img_in[iny * orig_w + inx];
+ } else {
+ img_out[i * out_w + j] = fill_value;
+ }
+ }
+ }
+
+ delete[] coords;
+ delete[] transformed_coords;
+ }
+}
+
+void imageRotate(double* img_in, int rows, int cols, double radians, double fill_value, double* img_out) {
+
+ // Translation matrix for moving the origin to the center of the image
+ double t1_data[] = {
+ 1, 0, static_cast<double>(-cols)/2,
+ 0, 1, static_cast<double>(-rows)/2,
+ 0, 0, 1
+ };
+ double* t1 = t1_data;
+ // Translation matrix for moving the origin back to the top left corner
+ double t2_data[] = {
+ 1, 0, static_cast<double>(cols)/2,
+ 0, 1, static_cast<double>(rows)/2,
+ 0, 0, 1
+ };
+ double* t2 = t2_data;
+ // The rotation matrix around the origin
+ double rot_data[] = {
+ cos(radians), sin(radians), 0,
+ -sin(radians), cos(radians), 0,
+ 0, 0, 1
+ };
+ double* rot = rot_data;
+
+ // Combined transformation matrix
+ double m_data1[3*3];
+ cblas_dgemm(CblasRowMajor, CblasNoTrans, CblasNoTrans, 3, 3, 3, 1.0, t2, 3, rot, 3, 0.0, m_data1, 3);
+ double m_data2[3*3];
+ cblas_dgemm(CblasRowMajor, CblasNoTrans, CblasNoTrans, 3, 3, 3, 1.0, m_data1, 3, t1, 3, 0.0, m_data2, 3);
+ double* m = m_data2;
+ // Transform image
+ img_transform(img_in,rows,cols,rows,cols,m[0], m[1], m[2], m[3], m[4], m[5], fill_value, img_out);
+}
+
+double* imageCutout(double* img_in, int rows, int cols, int x, int y, int width, int height, double fill_value) {
+ // Allocate memory for the output image using MKL
+ double* img_out = new double[rows * cols];
+
+ if (width < 1 || height < 1) {
+ // Invalid width or height, return the input image as it is
+ cblas_dcopy(rows * cols, img_in, 1, img_out, 1);
+ } else {
+ int end_x = x + width - 1;
+ int end_y = y + height - 1;
+
+ int start_x = std::max(1, x);
+ int start_y = std::max(1, y);
+ end_x = std::min(cols, end_x);
+ end_y = std::min(rows, end_y);
+
+ // Copy the input image to the output image using MKL
+ cblas_dcopy(rows * cols, img_in, 1, img_out, 1);
+
+ // Fill the cutout region with the fill_value
+ for (int i = start_y - 1; i < end_y; ++i) {
+ for (int j = start_x - 1; j < end_x; ++j) {
+ img_out[i * cols + j] = fill_value;
+ }
+ }
+ }
+
+ return img_out;
+}
+
+double* imageCrop(double* img_in, int orig_w, int orig_h, int w, int h, int x_offset, int y_offset) {
+ // Allocate memory for the output image
+ double* img_out = new double[w * h];
+
+ int start_h = (std::ceil((orig_h - h) / 2)) + y_offset - 1 ;
+ int end_h = (start_h + h - 1);
+ int start_w = (std::ceil((orig_w - w) / 2)) + x_offset - 1;
+ int end_w = (start_w + w - 1);
+
+ // Create a mask to identify the cropped region
+ double* mask = new double[orig_w * orig_h];
+ double* temp_mask = new double[w * h];
+
+ // Set mask elements to 0 outside the cropped region and 1 inside
+ memset(mask, 0, orig_w * orig_h * sizeof(double));
+ for(int i = 0; i < h * w; i++) {
+ temp_mask[i] = 1;
+ }
+
+ for (int i = start_h; i <= end_h; ++i) {
+ for (int j = start_w; j <= end_w; ++j) {
+ mask[i * orig_w + j] = temp_mask[(i - start_h) * w + (j - start_w)];
+ }
+ }
+
+ // Apply the mask to crop the image
+ for (int i = 0; i < h; ++i) {
+ for (int j = 0; j < w; ++j) {
+ img_out[i * w + j] = img_in[(start_h + i) * orig_w + (start_w + j)] * mask[(start_h + i) * orig_w + (start_w + j)];
+ }
+ }
+
+ // Free memory for the mask
+ delete[] mask;
+ delete[] temp_mask;
+
+ return img_out;
+}
+
+void img_translate(double* img_in, double offset_x, double offset_y,
+ int in_w, int in_h, int out_w, int out_h, double fill_value, double* img_out) {
+ int w = out_w;
+ int h = out_h;
+
+ offset_x = round(offset_x);
+ offset_y = round(offset_y);
+
+
+ int start_x = 0 - static_cast<int>(offset_x);
+ int start_y = 0 - static_cast<int>(offset_y);
+ int end_x = std::max(w, out_w) - static_cast<int>(offset_x);
+ int end_y = std::max(h, out_h) - static_cast<int>(offset_y);
+
+ if (start_x < 0)
+ start_x = 0;
+ if (start_y < 0)
+ start_y = 0;
+
+ if (w < end_x)
+ end_x = w;
+ if (h < end_y)
+ end_y = h;
+
+ if (out_w < end_x + static_cast<int>(offset_x))
+ end_x = out_w - static_cast<int>(offset_x);
+ if (out_h < end_y + static_cast<int>(offset_y))
+ end_y = out_h - static_cast<int>(offset_y);
+
+ for (int y = 0; y < out_h; ++y) {
+ for (int x = 0; x < out_w; ++x) {
+ img_out[y * out_w + x] = fill_value;
+ }
+ }
+
+ if (start_x < end_x && start_y < end_y) {
+ for (int y = start_y + static_cast<int>(offset_y); y < end_y + static_cast<int>(offset_y); ++y) {
+ int x_in = (start_x > 0) ? start_x + static_cast<int>(offset_x): start_x;
+ int y_in = (start_y > 0 ) ? y : y - static_cast<int>(offset_y);
+ cblas_dcopy(end_x - start_x, &img_in[(x_in) + (y_in) * in_w],
+ 1, &img_out[y * out_w + start_x + static_cast<int>(offset_x)], 1);
+ }
+ }
Review Comment:
indentation.
##########
src/main/cpp/imgUtils.h:
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.
+ */
+
+#ifndef IMAGE_UTILS_H
+#define IMAGE_UTILS_H
+#include <cmath>
+
+void printImage(const double* image, int rows, int cols);
Review Comment:
consider removing
##########
src/main/cpp/imgUtils.cpp:
##########
@@ -0,0 +1,243 @@
+/*
+ * 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.
+ */
+
+#include <iostream>
+#include <cmath>
+#include <cstring>
+#include "common.h"
+
+using namespace std;
+
+void printImage(const double* image, int rows, int cols) {
+ for (int i = 0; i < rows; i++) {
+ for (int j = 0; j < cols; j++) {
+ cout << image[i * cols + j] << " ";
+ }
+ cout << endl;
+ }
+ cout << "\n"<< endl;
+}
+
+void img_transform(const double* img_in, int orig_w, int orig_h, int out_w, int out_h, double a, double b, double c, double d,
+ double e, double f, double fill_value, double* img_out) {
+ double divisor = a * e - b * d;
+ if (divisor == 0) {
+ std::cout << "Inverse matrix does not exist! Returning input." << std::endl;
+ for (int i = 0; i < orig_h; i++) {
+ for (int j = 0; j < orig_w; j++) {
+ img_out[i * orig_w + j] = img_in[i * orig_w + j];
+ }
+ }
+ } else {
+ // Create the inverted transformation matrix
+ double T_inv[9];
+ T_inv[0] = e / divisor;
+ T_inv[1] = -b / divisor;
+ T_inv[2] = (b * f - c * e) / divisor;
+ T_inv[3] = -d / divisor;
+ T_inv[4] = a / divisor;
+ T_inv[5] = (c * d - a * f) / divisor;
+ T_inv[6] = 0.0;
+ T_inv[7] = 0.0;
+ T_inv[8] = 1.0;
+
+ // Create the coordinates of output pixel centers linearized in row-major order
+ double* coords = new double[2 * out_w * out_h];
+ for (int i = 0; i < out_h; i++) {
+ for (int j = 0; j < out_w; j++) {
+ coords[2 * (i * out_w + j)] = j + 0.5;
+ coords[2 * (i * out_w + j) + 1] = i + 0.5;
+ }
+ }
+
+ // Perform matrix multiplication to compute sampling pixel indices
+ double* transformed_coords = new double[2 * out_w * out_h];
Review Comment:
i do not know, but
can this be allocated as int?
##########
src/test/java/org/apache/sysds/test/functions/nativ/PerformanceComparison.java:
##########
@@ -0,0 +1,186 @@
+/*
+ * 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.test.functions.nativ;
+
+import org.apache.sysds.test.AutomatedTestBase;
+import org.apache.sysds.test.TestConfiguration;
+import org.apache.sysds.test.TestUtils;
+import org.apache.sysds.utils.ImgNativeHelper;
+import org.junit.Test;
+
+import java.util.Random;
+
+import static org.apache.sysds.utils.ImgNativeHelper.*;
Review Comment:
no wildcard imports allowed.
##########
src/test/java/org/apache/sysds/test/functions/nativ/ImgUtilsTest.java:
##########
@@ -0,0 +1,521 @@
+/*
+ * 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.test.functions.nativ;
+
+import org.apache.sysds.utils.ImgNativeHelper;
+import org.junit.Test;
+import static org.junit.Assert.assertArrayEquals;
+
+public class ImgUtilsTest {
+
+ @Test
+ public void runTestsWithOpenBlas() {
+ runTests("openblas");
+ }
+
+ //TODO configure intel mkl in test docker container to run this test
+ public void runTestsWithMKL() {
+ runTests("mkl");
+ }
+
+
+
+ public static void testImageRotation90And45(ImgNativeHelper imgNativeHelper) {
+
+ // Input image dimensions
+ int rows = 3;
+ int cols = 3;
+ // Input image
+ double[] img_in = {
+ 1,2,3,
+ 4,5,6,
+ 7,8,9
+ };
+ // Rotation angle in radians
+ double radians = Math.PI / 2;
+ // Fill value for the output image
+ double fill_value = 0.0;
+ // Expected output image
+ double[] expected_img_out_90 = {
+ 3,6,9,
+ 2,5,8,
+ 1,4,7
+ };
+
+ double[] expected_img_out_45 = {
+ 2,3,6,
+ 1,5,9,
+ 4,7,8
+ };
+
+ // Create the output image array
+ double[] img_out = new double[rows * cols];
+ imgNativeHelper.imageRotate(img_in, rows, cols, radians, fill_value, img_out);
+ assertArrayEquals(expected_img_out_90, img_out, 0.0001);
+ //rotate by 45
+ imgNativeHelper.imageRotate(img_in, rows, cols, radians/2, fill_value, img_out);
+ assertArrayEquals(expected_img_out_45, img_out, 0.0001);
+ }
+
+
+
+ public static void testImageRotation90And45_4x4(ImgNativeHelper imgNativeHelper) {
+
+ // Input image dimensions
+ int rows = 4;
+ int cols = 4;
+ // Input image
+ double[] img_in = {
+ 1,2,3,5,
+ 4,5,6,5,
+ 7,8,9,5,
+ 5,5,5,5,
+ };
+ // Rotation angle in radians
+ double radians = Math.PI / 2;
+ // Fill value for the output image
+ double fill_value = 0.0;
+ // Expected output image
+ double[] expected_img_out_90 = {
+ 5,5,5,5,
+ 3,6,9,5,
+ 2,5,8,5,
+ 1,4,7,5
+ };
+
+
+ double[] expected_img_out_45 = {
+ 0,3,5,0,
+ 2,5,6,5,
+ 4,5,8,5,
+ 0,7,5,0
+ };
+
+ // Create the output image array
+ double[] img_out = new double[rows * cols];
+ imgNativeHelper.imageRotate(img_in, rows, cols, radians, fill_value, img_out);
+ assertArrayEquals(expected_img_out_90, img_out, 0.0001);
+ //rotate by 45
+ imgNativeHelper.imageRotate(img_in, rows, cols, radians/2, fill_value, img_out);
+ assertArrayEquals(expected_img_out_45, img_out, 0.0001);
+ }
+
+
+ public static void testImageRotation45(ImgNativeHelper imgNativeHelper) {
+ // Input image dimensions
+ int rows = 6;
+ int cols = 6;
+
+ // Input image
+ double[] img_in = {
+ 1.0, 2.0, 3.0, 4.0,5.0,6.0,
+ 5.0, 6.0, 7.0, 8.0,9.0,10.0,
+ 9.0, 10.0, 11.0, 12.0,13.0,14.0,
+ 13.0, 14.0, 15.0, 16.0,17.0,18.0,
+ 17.0, 18.0, 19.0, 20.0, 21.0, 22.0,
+ 21.0,22.0,23.0,24.0,25.0,26.0
+ };
+
+ // Rotation angle in radians
+ double radians = Math.PI / 4;
+
+ // Fill value for the output image
+ double fill_value = 0.0;
+
+ // Expected output image
+ double[] expected_img_out = {
+ 0, 4, 5, 10 ,14, 0,
+ 3, 4, 8 ,13 ,18 ,18,
+ 2, 7, 12 ,16 ,17 ,22,
+ 5, 10, 15 ,16, 20 ,25,
+ 9, 13, 14, 19, 24 ,24,
+ 0, 13, 17 ,22, 23 ,0
+ };
+
+ // Create the output image array
+ double[] img_out = new double[rows * cols];
+
+ // Rotate the image
+ imgNativeHelper.imageRotate(img_in, rows, cols, radians, fill_value, img_out);
+
+ // Compare the output image with the expected image
+ assertArrayEquals(expected_img_out, img_out, 0.0001);
+ }
+
+
+ public static void testImageRotation180(ImgNativeHelper imgNativeHelper) {
+ // Input image dimensions
+ int rows = 4;
+ int cols = 4;
+
+ // Input image
+ double[] img_in = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 6.0, 7.0, 8.0,
+ 9.0, 10.0, 11.0, 12.0,
+ 13.0, 14.0, 15.0, 16.0
+ };
+
+ // Rotation angle in radians
+ double radians = Math.PI;
+
+ // Fill value for the output image
+ double fill_value = 0.0;
+
+ // Expected output image
+ double[] expected_img_out = {
+ 16.0, 15.0, 14.0, 13.0,
+ 12.0, 11.0, 10.0, 9.0,
+ 8.0, 7.0, 6.0, 5.0,
+ 4.0, 3.0, 2.0, 1.0
+ };
+
+ // Create the output image array
+ double[] img_out = new double[rows * cols];
+
+ // Rotate the image
+ imgNativeHelper.imageRotate(img_in, rows, cols, radians, fill_value, img_out);
+
+ // Compare the output image with the expected image
+ assertArrayEquals(expected_img_out, img_out, 0.0001);
+ }
+
+
+ public static void testCutoutImage(ImgNativeHelper imgNativeHelper) {
+ // Example input 2D matrix
+ double[] img_in = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 6.0, 7.0, 8.0,
+ 9.0, 10.0, 11.0, 12.0,
+ 13.0, 14.0, 15.0, 16.0
+ };
+
+ int rows = 4;
+ int cols = 4;
+ int x = 2;
+ int y = 2;
+ int width = 3;
+ int height = 3;
+ double fill_value = 0.0;
+
+ // Perform image cutout using JNI
+ double[] img_out = imgNativeHelper.imageCutout(img_in, rows, cols, x, y, width, height, fill_value);
+
+ // Expected output image after cutout
+ double[] expectedOutput = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 0.0, 0.0, 0.0,
+ 9.0, 0.0, 0.0, 0.0,
+ 13.0, 0.0, 0.0, 0.0
+ };
+
+ // Check if the output image matches the expected output
+ assertArrayEquals(expectedOutput, img_out,0.0001);
+ }
+
+
+ public static void testCutoutImageNonSquare(ImgNativeHelper imgNativeHelper) {
+ // Example input 2D matrix
+ double[] img_in = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 6.0, 7.0, 8.0,
+ 9.0, 10.0, 11.0, 12.0
+ };
+
+ int rows = 3;
+ int cols = 4;
+ int x = 1;
+ int y = 1;
+ int width = 3;
+ int height = 2;
+ double fill_value = 0.0;
+
+ // Perform image cutout using JNI
+ double[] img_out = imgNativeHelper.imageCutout(img_in, rows, cols, x, y, width, height, fill_value);
+
+ // Expected output image after cutout
+ double[] expectedOutput = {
+ 0.0, 0.0, 0.0, 4.0,
+ 0.0, 0.0, 0.0, 8.0,
+ 9.0, 10.0, 11.0, 12.0
+ };
+
+ // Check if the output image matches the expected output
+ assertArrayEquals(expectedOutput, img_out, 0.0001);
+ }
+
+
+ public static void testImageCutoutInvalidCutout(ImgNativeHelper imgNativeHelper) {
+ double[] img_in = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 6.0, 7.0, 8.0,
+ 9.0, 10.0, 11.0, 12.0,
+ 13.0, 14.0, 15.0, 16.0
+ };
+
+ int rows = 4;
+ int cols = 4;
+ int x = 3;
+ int y = 3;
+ int width = -2;
+ int height = 0;
+ double fill_value = 0.0;
+
+ double[] expectedOutput = img_in; // Expect no change since the cutout is invalid
+
+ double[] img_out = imgNativeHelper.imageCutout(img_in, rows, cols, x, y, width, height, fill_value);
+ assertArrayEquals(expectedOutput, img_out,0.0001);
+ }
+
+
+ public static void testImageCutoutNoCutout(ImgNativeHelper imgNativeHelper) {
+ double[] img_in = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 6.0, 7.0, 8.0,
+ 9.0, 10.0, 11.0, 12.0,
+ 13.0, 14.0, 15.0, 16.0
+ };
+
+ int rows = 4;
+ int cols = 4;
+ int x = 3;
+ int y = 3;
+ int width = 1;
+ int height = 1;
+ double fill_value = 0.0;
+
+ double[] expectedOutput = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 6.0, 7.0, 8.0,
+ 9.0, 10.0, 0.0, 12.0,
+ 13.0, 14.0, 15.0, 16.0
+ };
+
+ double[] img_out = imgNativeHelper.imageCutout(img_in, rows, cols, x, y, width, height, fill_value);
+ assertArrayEquals(expectedOutput, img_out,0.0001);
+ }
+
+
+ public static void testImageCropValidCrop(ImgNativeHelper imgNativeHelper) {
+ double[] img_in = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 6.0, 7.0, 8.0,
+ 9.0, 10.0, 11.0, 12.0,
+ 13.0, 14.0, 15.0, 16.0
+ };
+
+ int orig_w = 4;
+ int orig_h = 4;
+ int w = 2;
+ int h = 2;
+ int x_offset = 1;
+ int y_offset = 1;
+
+ double[] expectedOutput = {
+ 6.0, 7.0,
+ 10.0, 11.0
+ };
+
+ double[] img_out = imgNativeHelper.cropImage(img_in, orig_w, orig_h, w, h, x_offset, y_offset);
+
+ assertArrayEquals(expectedOutput, img_out,0.0001); }
+
+
+ public static void testImageCropInvalidCrop(ImgNativeHelper imgNativeHelper) {
+ double[] img_in = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 6.0, 7.0, 8.0,
+ 9.0, 10.0, 11.0, 12.0,
+ 13.0, 14.0, 15.0, 16.0
+ };
+
+ int orig_w = 4;
+ int orig_h = 4;
+ int w = 5;
+ int h = 5;
+ int x_offset = 1;
+ int y_offset = 1;
+
+ double[] expectedOutput = img_in; // Expect no change since the crop is invalid
+
+ double[] img_out = imgNativeHelper.cropImage(img_in, orig_w, orig_h, w, h, x_offset, y_offset);
+ assertArrayEquals(expectedOutput, img_out,0.0001);
+ }
+
+
+ public void testImgTranslate(ImgNativeHelper imgNativeHelper) {
+ int in_w = 5;
+ int in_h = 5;
+ int out_w = 7;
+ int out_h = 7;
+ double fill_value = 0.0;
+
+ double[] img_in = new double[in_w * in_h];
+ for (int i = 0; i < in_w * in_h; ++i) {
+ img_in[i] = i + 1; // Filling input image with sequential values
+ }
+
+ double[] img_out = new double[out_w * out_h];
+
+ double offset_x = 1.5;
+ double offset_y = 1.5;
+
+ imgNativeHelper.imgTranslate(img_in, offset_x, offset_y, in_w, in_h, out_w, out_h, fill_value, img_out);
+
+ // Expected output based on the given offsets and fill value
+ double[] expectedOutput = {
+ 0,0,0,0,0,0,0,
+ 0,0,0,0,0,0,0,
+ 0,0,1,2,3,4,5,
+ 0,0,6,7,8,9,10,
+ 0,0,11,12,13,14,15,
+ 0,0,16,17,18,19,20,
+ 0,0,21,22,23,24,25
+ };
+
+ assertArrayEquals(expectedOutput, img_out, 1e-9); // Compare arrays with a small epsilon
+ }
+
+
+ public static void testImgTranslateNegativeOffsets(ImgNativeHelper imgNativeHelper) {
+ int in_w = 5;
+ int in_h = 5;
+ int out_w = 6;
+ int out_h = 6;
+ double fill_value = 0.0;
+
+ double[] img_in = new double[in_w * in_h];
+ for (int i = 0; i < in_w * in_h; ++i) {
+ img_in[i] = i + 1; // Filling input image with sequential values
+ }
+
+ double[] img_out = new double[out_w * out_h];
+
+ double offset_x = -0.5; // Negative offset in X direction
+ double offset_y = -0.5; // Negative offset in Y direction
+
+ imgNativeHelper.imgTranslate(img_in, offset_x, offset_y, in_w, in_h, out_w, out_h, fill_value, img_out);
+
+ // Expected output based on the given offsets and fill value
+ double[] expectedOutput = {
+ 1,2,3,4,5,0,
+ 6,7,8,9,10,0,
+ 11,12,13,14,15,0,
+ 16,17,18,19,20,0,
+ 21,22,23,24,25,0,
+ 0,0,0,0,0,0,
+ };
+
+ assertArrayEquals(expectedOutput, img_out, 1e-9); // Compare arrays with a small epsilon
+ }
+
+
+ public static void testImgTranslatePositiveAndNegativeOffsets(ImgNativeHelper imgNativeHelper) {
+ int in_w = 5;
+ int in_h = 5;
+ int out_w = 6;
+ int out_h = 6;
+ double fill_value = 0.0;
+
+ double[] img_in = new double[in_w * in_h];
+ for (int i = 0; i < in_w * in_h; ++i) {
+ img_in[i] = i + 1; // Filling input image with sequential values
+ }
+
+ double[] img_out = new double[out_w * out_h];
+
+ double offset_x = -0.5; // Negative offset in X direction
+ double offset_y = 0.5; // Negative offset in Y direction
+
+ imgNativeHelper.imgTranslate(img_in, offset_x, offset_y, in_w, in_h, out_w, out_h, fill_value, img_out);
+
+ // Expected output based on the given offsets and fill value
+ double[] expectedOutput = {
+ 0,0,0,0,0,0,
+ 1,2,3,4,5,0,
+ 6,7,8,9,10,0,
+ 11,12,13,14,15,0,
+ 16,17,18,19,20,0,
+ 21,22,23,24,25,0,
+ };
+
+ assertArrayEquals(expectedOutput, img_out, 1e-9); // Compare arrays with a small epsilon
+ }
+
+
+ public static void testImgTranslatePositiveAndNegativeOffsets2(ImgNativeHelper imgNativeHelper) {
+ int in_w = 5;
+ int in_h = 5;
+ int out_w = 6;
+ int out_h = 6;
+ double fill_value = 0.0;
+
+ double[] img_in = new double[in_w * in_h];
+ for (int i = 0; i < in_w * in_h; ++i) {
+ img_in[i] = i + 1; // Filling input image with sequential values
+ }
+
+ double[] img_out = new double[out_w * out_h];
+
+ double offset_x = 0.5; // Negative offset in X direction
+ double offset_y = -0.5; // Negative offset in Y direction
+
+ imgNativeHelper.imgTranslate(img_in, offset_x, offset_y, in_w, in_h, out_w, out_h, fill_value, img_out);
+
+ // Expected output based on the given offsets and fill value
+ double[] expectedOutput = {
+ 0,1,2,3,4,5,
+ 0,6,7,8,9,10,
+ 0,11,12,13,14,15,
+ 0,16,17,18,19,20,
+ 0,21,22,23,24,25,
+ 0,0,0,0,0,0,
+ };
+
+ assertArrayEquals(expectedOutput, img_out, 1e-9); // Compare arrays with a small epsilon
+ }
+
+ public void runTests(String blasType) {
Review Comment:
i understand why you designed it this way, but please make the test integrate with JUnit,
To do this remove the `runTests` method and make all your static calls annotated with @ Test.
Then make the imgNativeHelper a field in this class that the constructor instantiate.
Then finally the class needs to be changed into a parameterized class, that calls the constructor selecting either BLAS or MKL.
##########
src/main/cpp/systemds_img.cpp:
##########
@@ -0,0 +1,135 @@
+/*
+ * 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.
+ */
+
+#ifdef _WIN32
+#include <winsock.h>
+#else
+#include <arpa/inet.h>
+#endif
+
+#include "common.h"
+#include "libmatrixdnn.h"
+#include "libmatrixmult.h"
+#include "systemds.h"
+#include "imgUtils.h"
+
+JNIEXPORT void JNICALL Java_org_apache_sysds_utils_ImgNativeHelper_imageRotate
+ (JNIEnv* env, jclass clazz, jdoubleArray img_in, jint rows, jint cols, jdouble radians, jdouble fill_value, jdoubleArray img_out) {
+
+ if (rows != cols) {
+ jclass exceptionClass = env->FindClass("java/lang/IllegalArgumentException");
+ if (exceptionClass != NULL) {
+ env->ThrowNew(exceptionClass, "Image dimensions must be equal");
+ return;
+ }
+ }
+
+ jsize num_pixels = env->GetArrayLength(img_in);
+ jdouble* img_in_data = env->GetDoubleArrayElements(img_in, NULL);
+ jdouble* img_out_data = new jdouble[num_pixels];
+ imageRotate(img_in_data, rows, cols, radians, fill_value, img_out_data);
+ env->SetDoubleArrayRegion(img_out, 0, num_pixels, img_out_data);
+ delete[] img_out_data;
+ env->ReleaseDoubleArrayElements(img_in, img_in_data, JNI_ABORT);
+}
+
+JNIEXPORT jdoubleArray JNICALL Java_org_apache_sysds_utils_ImgNativeHelper_imageCutout
+ (JNIEnv* env, jclass cls, jdoubleArray img_in, jint rows, jint cols, jint x, jint y, jint width, jint height, jdouble fill_value) {
+
+ jdouble* img_in_arr = env->GetDoubleArrayElements(img_in, nullptr);
+ jdouble* img_out_arr = imageCutout(img_in_arr, rows, cols, x, y, width, height, fill_value);
+ jdoubleArray img_out = env->NewDoubleArray(rows * cols);
+
+ env->SetDoubleArrayRegion(img_out, 0, rows * cols, img_out_arr);
+ env->ReleaseDoubleArrayElements(img_in, img_in_arr, JNI_ABORT);
+ delete[] img_out_arr;
+
+ return img_out;
+}
+
+JNIEXPORT jdoubleArray JNICALL Java_org_apache_sysds_utils_ImgNativeHelper_cropImage(JNIEnv *env, jclass,
+ jdoubleArray img_in, jint orig_w, jint orig_h, jint w, jint h, jint x_offset, jint y_offset) {
+ jsize length = env->GetArrayLength(img_in);
+ double *img_in_array = env->GetDoubleArrayElements(img_in, 0);
+
+ int start_h = (ceil((orig_h - h) / 2)) + y_offset - 1;
+ int end_h = (start_h + h - 1);
+ int start_w = (ceil((orig_w - w) / 2)) + x_offset - 1;
+ int end_w = (start_w + w - 1);
+
+ jdoubleArray img_out_java;
+ if (start_h < 0 || end_h >= orig_h || start_w < 0 || end_w >= orig_w) {
+ img_out_java = env->NewDoubleArray(orig_w * orig_h);
+ env->SetDoubleArrayRegion(img_out_java, 0, orig_w * orig_h, img_in_array);
+
+ }else {
+ double *img_out = imageCrop(img_in_array, orig_w, orig_h, w, h, x_offset, y_offset);
+
+ if (img_out == nullptr) {
+ return nullptr;
+ }
+
+ img_out_java = env->NewDoubleArray(w * h);
+ env->SetDoubleArrayRegion(img_out_java, 0, w * h, img_out);
+ delete[] img_out;
+ }
+
+ env->ReleaseDoubleArrayElements(img_in, img_in_array, 0);
+
+ return img_out_java;
+}
+
+/*JNIEXPORT jdoubleArray JNICALL Java_org_apache_sysds_utils_ImgNativeHelper_shearImage(JNIEnv *env, jclass,
+ jdoubleArray img_in, jint width, jint height, jdouble shear_x, jdouble shear_y, jdouble fill_value) {
+
+
+ jsize length = env->GetArrayLength(img_in);
+ double *img_in_array = env->GetDoubleArrayElements(img_in, 0);
+
+
+ double* img_out = img_transform(img_in_array, width, height, shear_x, shear_y, fill_value);
+
+
+ env->ReleaseDoubleArrayElements(img_in, img_in_array, 0);
+
+ if (img_out == nullptr) {
+ return nullptr;
+ }
+
+
+ jdoubleArray img_out_java = env->NewDoubleArray(width * height);
+ env->SetDoubleArrayRegion(img_out_java, 0, width * height, img_out);
+
+
+ delete[] img_out;
+
+ return img_out_java;
+}*/
+
+JNIEXPORT void JNICALL Java_org_apache_sysds_utils_ImgNativeHelper_imgTranslate(JNIEnv *env, jclass cls,
+ jdoubleArray img_in, jdouble offset_x, jdouble offset_y,
+ jint in_w, jint in_h, jint out_w, jint out_h,
+ jdouble fill_value, jdoubleArray img_out) {
+
+ jdouble* j_img_in = env->GetDoubleArrayElements(img_in, nullptr);
+ jdouble* j_img_out = env->GetDoubleArrayElements(img_out, nullptr);
+ img_translate(j_img_in, offset_x, offset_y, in_w, in_h, out_w, out_h, fill_value, j_img_out);
+ env->ReleaseDoubleArrayElements(img_in, j_img_in, 0);
+ env->ReleaseDoubleArrayElements(img_out, j_img_out, 0);
+}
Review Comment:
newline in the end.
##########
src/main/java/org/apache/sysds/utils/ImgNativeHelper.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * 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.utils;
+
+import org.apache.commons.lang.SystemUtils;
+
+import java.io.File;
+
+public class ImgNativeHelper extends NativeHelper {
Review Comment:
you can add doc on the Helper class itself.
i would suggest a hint at where to find the CPP files.
##########
src/main/cpp/imgUtils.cpp:
##########
@@ -0,0 +1,243 @@
+/*
+ * 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.
+ */
+
+#include <iostream>
+#include <cmath>
+#include <cstring>
+#include "common.h"
+
+using namespace std;
+
+void printImage(const double* image, int rows, int cols) {
+ for (int i = 0; i < rows; i++) {
+ for (int j = 0; j < cols; j++) {
+ cout << image[i * cols + j] << " ";
+ }
+ cout << endl;
+ }
+ cout << "\n"<< endl;
+}
+
+void img_transform(const double* img_in, int orig_w, int orig_h, int out_w, int out_h, double a, double b, double c, double d,
+ double e, double f, double fill_value, double* img_out) {
+ double divisor = a * e - b * d;
+ if (divisor == 0) {
+ std::cout << "Inverse matrix does not exist! Returning input." << std::endl;
+ for (int i = 0; i < orig_h; i++) {
+ for (int j = 0; j < orig_w; j++) {
+ img_out[i * orig_w + j] = img_in[i * orig_w + j];
+ }
+ }
+ } else {
+ // Create the inverted transformation matrix
+ double T_inv[9];
+ T_inv[0] = e / divisor;
+ T_inv[1] = -b / divisor;
+ T_inv[2] = (b * f - c * e) / divisor;
+ T_inv[3] = -d / divisor;
+ T_inv[4] = a / divisor;
+ T_inv[5] = (c * d - a * f) / divisor;
+ T_inv[6] = 0.0;
+ T_inv[7] = 0.0;
+ T_inv[8] = 1.0;
+
+ // Create the coordinates of output pixel centers linearized in row-major order
+ double* coords = new double[2 * out_w * out_h];
+ for (int i = 0; i < out_h; i++) {
+ for (int j = 0; j < out_w; j++) {
+ coords[2 * (i * out_w + j)] = j + 0.5;
+ coords[2 * (i * out_w + j) + 1] = i + 0.5;
+ }
+ }
+
+ // Perform matrix multiplication to compute sampling pixel indices
+ double* transformed_coords = new double[2 * out_w * out_h];
+ for (int i = 0; i < out_w * out_h; i++) {
+ double x = coords[2 * i];
+ double y = coords[2 * i + 1];
+ transformed_coords[2 * i] = std::floor(T_inv[0] * x + T_inv[1] * y + T_inv[2]) + 1;
+ transformed_coords[2 * i + 1] = std::floor(T_inv[3] * x + T_inv[4] * y + T_inv[5]) + 1;
+ }
+
+ // Fill output image
+ for (int i = 0; i < out_h; i++) {
+ for (int j = 0; j < out_w; j++) {
+ int inx = static_cast<int>(transformed_coords[2 * (i * out_w + j)]) - 1;
+ int iny = static_cast<int>(transformed_coords[2 * (i * out_w + j) + 1]) - 1;
+ if (inx >= 0 && inx < orig_w && iny >= 0 && iny < orig_h) {
+ img_out[i * out_w + j] = img_in[iny * orig_w + inx];
+ } else {
+ img_out[i * out_w + j] = fill_value;
+ }
+ }
+ }
+
+ delete[] coords;
+ delete[] transformed_coords;
+ }
+}
+
+void imageRotate(double* img_in, int rows, int cols, double radians, double fill_value, double* img_out) {
+
+ // Translation matrix for moving the origin to the center of the image
+ double t1_data[] = {
+ 1, 0, static_cast<double>(-cols)/2,
+ 0, 1, static_cast<double>(-rows)/2,
+ 0, 0, 1
+ };
+ double* t1 = t1_data;
+ // Translation matrix for moving the origin back to the top left corner
+ double t2_data[] = {
+ 1, 0, static_cast<double>(cols)/2,
+ 0, 1, static_cast<double>(rows)/2,
+ 0, 0, 1
+ };
+ double* t2 = t2_data;
+ // The rotation matrix around the origin
+ double rot_data[] = {
+ cos(radians), sin(radians), 0,
+ -sin(radians), cos(radians), 0,
+ 0, 0, 1
+ };
+ double* rot = rot_data;
+
+ // Combined transformation matrix
+ double m_data1[3*3];
+ cblas_dgemm(CblasRowMajor, CblasNoTrans, CblasNoTrans, 3, 3, 3, 1.0, t2, 3, rot, 3, 0.0, m_data1, 3);
+ double m_data2[3*3];
+ cblas_dgemm(CblasRowMajor, CblasNoTrans, CblasNoTrans, 3, 3, 3, 1.0, m_data1, 3, t1, 3, 0.0, m_data2, 3);
+ double* m = m_data2;
+ // Transform image
+ img_transform(img_in,rows,cols,rows,cols,m[0], m[1], m[2], m[3], m[4], m[5], fill_value, img_out);
+}
+
+double* imageCutout(double* img_in, int rows, int cols, int x, int y, int width, int height, double fill_value) {
+ // Allocate memory for the output image using MKL
+ double* img_out = new double[rows * cols];
+
+ if (width < 1 || height < 1) {
+ // Invalid width or height, return the input image as it is
+ cblas_dcopy(rows * cols, img_in, 1, img_out, 1);
+ } else {
+ int end_x = x + width - 1;
+ int end_y = y + height - 1;
+
+ int start_x = std::max(1, x);
+ int start_y = std::max(1, y);
+ end_x = std::min(cols, end_x);
+ end_y = std::min(rows, end_y);
+
+ // Copy the input image to the output image using MKL
+ cblas_dcopy(rows * cols, img_in, 1, img_out, 1);
+
+ // Fill the cutout region with the fill_value
+ for (int i = start_y - 1; i < end_y; ++i) {
+ for (int j = start_x - 1; j < end_x; ++j) {
+ img_out[i * cols + j] = fill_value;
+ }
+ }
+ }
+
+ return img_out;
+}
+
+double* imageCrop(double* img_in, int orig_w, int orig_h, int w, int h, int x_offset, int y_offset) {
+ // Allocate memory for the output image
+ double* img_out = new double[w * h];
+
+ int start_h = (std::ceil((orig_h - h) / 2)) + y_offset - 1 ;
+ int end_h = (start_h + h - 1);
+ int start_w = (std::ceil((orig_w - w) / 2)) + x_offset - 1;
+ int end_w = (start_w + w - 1);
+
+ // Create a mask to identify the cropped region
+ double* mask = new double[orig_w * orig_h];
+ double* temp_mask = new double[w * h];
+
+ // Set mask elements to 0 outside the cropped region and 1 inside
+ memset(mask, 0, orig_w * orig_h * sizeof(double));
+ for(int i = 0; i < h * w; i++) {
+ temp_mask[i] = 1;
+ }
+
+ for (int i = start_h; i <= end_h; ++i) {
+ for (int j = start_w; j <= end_w; ++j) {
+ mask[i * orig_w + j] = temp_mask[(i - start_h) * w + (j - start_w)];
+ }
+ }
+
+ // Apply the mask to crop the image
+ for (int i = 0; i < h; ++i) {
+ for (int j = 0; j < w; ++j) {
+ img_out[i * w + j] = img_in[(start_h + i) * orig_w + (start_w + j)] * mask[(start_h + i) * orig_w + (start_w + j)];
+ }
+ }
+
+ // Free memory for the mask
+ delete[] mask;
+ delete[] temp_mask;
+
+ return img_out;
+}
+
+void img_translate(double* img_in, double offset_x, double offset_y,
Review Comment:
cast the offsets in the first line rather than every call to offset_x and y, and maybe consider changing the API to use int offsets.
##########
src/main/cpp/systemds_img.cpp:
##########
@@ -0,0 +1,135 @@
+/*
+ * 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.
+ */
+
+#ifdef _WIN32
+#include <winsock.h>
+#else
+#include <arpa/inet.h>
+#endif
+
+#include "common.h"
+#include "libmatrixdnn.h"
+#include "libmatrixmult.h"
+#include "systemds.h"
+#include "imgUtils.h"
+
+JNIEXPORT void JNICALL Java_org_apache_sysds_utils_ImgNativeHelper_imageRotate
+ (JNIEnv* env, jclass clazz, jdoubleArray img_in, jint rows, jint cols, jdouble radians, jdouble fill_value, jdoubleArray img_out) {
+
+ if (rows != cols) {
+ jclass exceptionClass = env->FindClass("java/lang/IllegalArgumentException");
+ if (exceptionClass != NULL) {
+ env->ThrowNew(exceptionClass, "Image dimensions must be equal");
+ return;
+ }
+ }
+
+ jsize num_pixels = env->GetArrayLength(img_in);
+ jdouble* img_in_data = env->GetDoubleArrayElements(img_in, NULL);
+ jdouble* img_out_data = new jdouble[num_pixels];
+ imageRotate(img_in_data, rows, cols, radians, fill_value, img_out_data);
+ env->SetDoubleArrayRegion(img_out, 0, num_pixels, img_out_data);
+ delete[] img_out_data;
+ env->ReleaseDoubleArrayElements(img_in, img_in_data, JNI_ABORT);
+}
+
+JNIEXPORT jdoubleArray JNICALL Java_org_apache_sysds_utils_ImgNativeHelper_imageCutout
+ (JNIEnv* env, jclass cls, jdoubleArray img_in, jint rows, jint cols, jint x, jint y, jint width, jint height, jdouble fill_value) {
+
+ jdouble* img_in_arr = env->GetDoubleArrayElements(img_in, nullptr);
+ jdouble* img_out_arr = imageCutout(img_in_arr, rows, cols, x, y, width, height, fill_value);
+ jdoubleArray img_out = env->NewDoubleArray(rows * cols);
+
+ env->SetDoubleArrayRegion(img_out, 0, rows * cols, img_out_arr);
+ env->ReleaseDoubleArrayElements(img_in, img_in_arr, JNI_ABORT);
+ delete[] img_out_arr;
+
+ return img_out;
+}
+
+JNIEXPORT jdoubleArray JNICALL Java_org_apache_sysds_utils_ImgNativeHelper_cropImage(JNIEnv *env, jclass,
+ jdoubleArray img_in, jint orig_w, jint orig_h, jint w, jint h, jint x_offset, jint y_offset) {
+ jsize length = env->GetArrayLength(img_in);
+ double *img_in_array = env->GetDoubleArrayElements(img_in, 0);
+
+ int start_h = (ceil((orig_h - h) / 2)) + y_offset - 1;
+ int end_h = (start_h + h - 1);
+ int start_w = (ceil((orig_w - w) / 2)) + x_offset - 1;
+ int end_w = (start_w + w - 1);
+
+ jdoubleArray img_out_java;
+ if (start_h < 0 || end_h >= orig_h || start_w < 0 || end_w >= orig_w) {
+ img_out_java = env->NewDoubleArray(orig_w * orig_h);
+ env->SetDoubleArrayRegion(img_out_java, 0, orig_w * orig_h, img_in_array);
+
+ }else {
+ double *img_out = imageCrop(img_in_array, orig_w, orig_h, w, h, x_offset, y_offset);
+
+ if (img_out == nullptr) {
+ return nullptr;
+ }
+
+ img_out_java = env->NewDoubleArray(w * h);
+ env->SetDoubleArrayRegion(img_out_java, 0, w * h, img_out);
+ delete[] img_out;
+ }
+
+ env->ReleaseDoubleArrayElements(img_in, img_in_array, 0);
+
+ return img_out_java;
+}
+
+/*JNIEXPORT jdoubleArray JNICALL Java_org_apache_sysds_utils_ImgNativeHelper_shearImage(JNIEnv *env, jclass,
+ jdoubleArray img_in, jint width, jint height, jdouble shear_x, jdouble shear_y, jdouble fill_value) {
+
+
+ jsize length = env->GetArrayLength(img_in);
+ double *img_in_array = env->GetDoubleArrayElements(img_in, 0);
+
+
+ double* img_out = img_transform(img_in_array, width, height, shear_x, shear_y, fill_value);
+
+
+ env->ReleaseDoubleArrayElements(img_in, img_in_array, 0);
+
+ if (img_out == nullptr) {
+ return nullptr;
+ }
+
+
+ jdoubleArray img_out_java = env->NewDoubleArray(width * height);
+ env->SetDoubleArrayRegion(img_out_java, 0, width * height, img_out);
+
+
+ delete[] img_out;
+
+ return img_out_java;
+}*/
Review Comment:
remove commented code.
##########
src/main/cpp/build.sh:
##########
@@ -46,6 +47,14 @@ if ! [ -x "$(command -v patchelf)" ]; then
exit 1
fi
+# Check if OpenBLAS is installed
+if ! ldconfig -p | grep -q libopenblas; then
+ echo "OpenBLAS not found. Installing OpenBLAS..."
+
+ apt-get update
+ apt-get install libopenblas-dev -y
+fi
Review Comment:
add comment on MKL install.
##########
src/main/cpp/lib/libsystemds_mkl-Linux-x86_64.so:
##########
Review Comment:
maybe not add these?
##########
src/main/cpp/imgUtils.cpp:
##########
@@ -0,0 +1,243 @@
+/*
+ * 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.
+ */
+
+#include <iostream>
+#include <cmath>
+#include <cstring>
+#include "common.h"
+
+using namespace std;
+
+void printImage(const double* image, int rows, int cols) {
+ for (int i = 0; i < rows; i++) {
+ for (int j = 0; j < cols; j++) {
+ cout << image[i * cols + j] << " ";
+ }
+ cout << endl;
+ }
+ cout << "\n"<< endl;
+}
Review Comment:
i suggest removing this debugging printImage.
##########
src/main/java/org/apache/sysds/utils/ImgNativeHelper.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * 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.utils;
+
+import org.apache.commons.lang.SystemUtils;
+
+import java.io.File;
+
+public class ImgNativeHelper extends NativeHelper {
+ private final String blasType;
+
+ public ImgNativeHelper(String blasType) {
+ if(blasType != null) this.blasType = blasType;
+ else this.blasType = "openblas";
+ loadNativeLibrary();
+ }
+
+ // Method to load the native library based on the specified BLAS type
+ private void loadNativeLibrary() {
+ try {
+ if (SystemUtils.IS_OS_LINUX) {
+ String libname = blasType + "-Linux-x86_64.so";
+ System.load(System.getProperty("user.dir")+"/src/main/cpp/lib/libsystemds_" + libname);
+ //System.load("/home/runner/work/systemds/systemds/src/main/cpp/lib/libsystemds_openblas-Linux-x86_64.so");
+ } else if (SystemUtils.IS_OS_WINDOWS) {
+ String libname = blasType + "-Windows-x86_64.dll";
+ System.load("/src/main/cpp/lib/".replace("/", File.separator) + "libsystemds_" + libname);
+ } else if (SystemUtils.IS_OS_MAC || SystemUtils.IS_OS_MAC_OSX) {
+ // String libname = "systemds_" + blasType + "-Darwin-x86_64";
+ System.load(System.getProperty("user.dir")+"/src/main/cpp/lib/libsystemds_" + blasType + "-Darwin-x86_64.dylib");
Review Comment:
same here, remove commented code,
##########
src/main/java/org/apache/sysds/utils/ImgNativeHelper.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * 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.utils;
+
+import org.apache.commons.lang.SystemUtils;
+
+import java.io.File;
+
+public class ImgNativeHelper extends NativeHelper {
+ private final String blasType;
+
+ public ImgNativeHelper(String blasType) {
+ if(blasType != null) this.blasType = blasType;
+ else this.blasType = "openblas";
+ loadNativeLibrary();
+ }
+
+ // Method to load the native library based on the specified BLAS type
+ private void loadNativeLibrary() {
+ try {
+ if (SystemUtils.IS_OS_LINUX) {
+ String libname = blasType + "-Linux-x86_64.so";
+ System.load(System.getProperty("user.dir")+"/src/main/cpp/lib/libsystemds_" + libname);
+ //System.load("/home/runner/work/systemds/systemds/src/main/cpp/lib/libsystemds_openblas-Linux-x86_64.so");
+ } else if (SystemUtils.IS_OS_WINDOWS) {
+ String libname = blasType + "-Windows-x86_64.dll";
+ System.load("/src/main/cpp/lib/".replace("/", File.separator) + "libsystemds_" + libname);
+ } else if (SystemUtils.IS_OS_MAC || SystemUtils.IS_OS_MAC_OSX) {
+ // String libname = "systemds_" + blasType + "-Darwin-x86_64";
+ System.load(System.getProperty("user.dir")+"/src/main/cpp/lib/libsystemds_" + blasType + "-Darwin-x86_64.dylib");
+ }
+ } catch (Exception e) {
+ e.printStackTrace();
Review Comment:
i would like this to throw the exception, not just print stack trace.
you can wrap the exception in a `throw new RuntimeException(e);`
##########
src/test/java/org/apache/sysds/test/functions/nativ/PerformanceComparison.java:
##########
@@ -0,0 +1,186 @@
+/*
+ * 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.test.functions.nativ;
+
+import org.apache.sysds.test.AutomatedTestBase;
+import org.apache.sysds.test.TestConfiguration;
+import org.apache.sysds.test.TestUtils;
+import org.apache.sysds.utils.ImgNativeHelper;
+import org.junit.Test;
+
+import java.util.Random;
+
+import static org.apache.sysds.utils.ImgNativeHelper.*;
+
+public class PerformanceComparison extends AutomatedTestBase {
+
+ private final static String TEST_DIR = "functions/builtin/";
+ private final static String TEST_CLASS_DIR = TEST_DIR + PerformanceComparison.class.getSimpleName() + "/";
+
+ private final static double eps = 1e-10;
+ private final static int rows = 512;
+ private final static int cols = 512;
+ private final static double spSparse = 0.1;
+ private final static double spDense = 0.9;
+ private final static int x_offset = 12;
+ private final static int y_offset = 24;
+ private final static float size = 0.8f;
+
+ private final ImgNativeHelper imgNativeHelper = new ImgNativeHelper("openblas");
+
+ @Override
+ public void setUp() {
+ // addTestConfiguration(TEST_NAME,new TestConfiguration(TEST_CLASS_DIR, TEST_NAME,new String[]{"B"}));
+ }
+
+ private static double randomDouble(double min, double max) {
+ return min + (max - min) * new Random().nextDouble();
+ }
+
+ // Function to generate a random square matrix of size n x n
+ public static double[] generateRandomMatrix(int rows, int cols, double min, double max, double sparsity, long seed) {
+ double[] matrix = new double[rows * cols];
+ Random random = (seed == -1) ? TestUtils.random : new Random(seed);
+
+ for (int i = 0; i < rows; i++) {
+ for (int j = 0; j < cols; j++) {
+ int index = i * cols + j; // Calculate the index for the 1D array
+ if (random.nextDouble() > sparsity) {
+ continue;
+ }
+ matrix[index] = (random.nextDouble() * (max - min) + min);
+ }
+ }
+
+ return matrix;
+ }
+
+ // Function to print a square matrix
+ private static void printMatrix(double[] matrix, int n) {
+ for (int i = 0; i < n; i++) {
+ for (int j = 0; j < n; j++) {
+ System.out.printf("%.4f\t", matrix[i * n + j]);
+ }
+ System.out.println();
+ }
+ }
+
+ public void runBlasTests(boolean sparse,int n, int seed) {
+ double spSparse = 0.1;
Review Comment:
indentation
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1722584515
<img width="771" alt="image" src="https://github.com/apache/systemds/assets/68165368/5c2c8459-2407-4dce-ab09-c6360fd13d70">
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1722584575
<img width="761" alt="image" src="https://github.com/apache/systemds/assets/68165368/29e18030-3840-4292-aa02-b4fab4797af7">
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1715455117
> Hi @Baunsgaard The build was successful for Linux using openblas, little progress. However I noticed that the built shared library location is different from the workspace. /home/runner/work/systemds/systemds/src/main/cpp/lib/libsystemds_openblas-Linux-x86_64.so
>
> I am trying again with this absolute path.
figuring out where all these things are, can be tricky. hope it works out it is a bit of trial and error.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] j143 commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "j143 (via GitHub)" <gi...@apache.org>.
j143 commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1719379663
Hi @anishsapkota -- you can go to actions tab in your personal fork (ex, https://github.com/anishsapkota/systemds/actions) and enable the workflows! you shall be able to run these tests there too! you can disable workflows that are not related to your changes.
also, you can [enable debug option](https://docs.github.com/en/actions/monitoring-and-troubleshooting-workflows/enabling-debug-logging), if required to get even more logs for you test case.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1719173164
@Baunsgaard upon checking the env variables, the LD_LIBRARY_PATH and LIBRARY_PATH both contains the path to the folder where libmkl_rt.so is.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1715637240
I was going through the failed javaTest, and apparently I am installing the openblas outside of the docker container, I put the install section in build.yml file https://github.com/apache/systemds/pull/1843/commits/c500e1192665f3bea8df0a88135b62857635add2
should I be putting install OpenBlas section in action.yml ?
-- Configuring incomplete, errors occurred!
See also "/github/workspace/src/main/cpp/INTEL/CMakeFiles/CMakeOutput.log".
make: *** No rule to make target 'install'. Stop.
-- The CXX compiler identification is GNU 9.4.0
-- Check for working CXX compiler: /usr/bin/g++
-- Check for working CXX compiler: /usr/bin/g++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Error at cmake/FindOpenBLAS.cmake:72 (MESSAGE):
-- Could not find OpenBLAS lib. Turning OpenBLAS_FOUND off
Could not find OpenBLAS
Call Stack (most recent call first):
CMakeLists.txt:69 (find_package)
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1714508113
https://github.com/apache/systemds/actions/runs/6151101437/job/16690515871?pr=1843
Some of the errors are to be expected Since some packages are missing, but it seems clear to me that it does not build the img packages. Could it be that we are missing the MKL and BLAS dependencies on the GitHub install?
if so is there some install sequence that can be run on Ubuntu, then we can make it work at least for Linux.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1721882836
> My main comment is the comparison is not fair. There are some things we can do to improve it if you want (read the detailed comments for more).
Hi @Baunsgaard ,
thank your for reviewing the code and the suggestions. Rest assured, I will address all of them after my presentation.
could you be more specific, how can I make the comparison fairer? I will be incorporating your previous suggestions of calculating std and analysing dml execution time using -stats flag. Do you have any further suggestions?
It would be interesting for the presentation to address this issue before 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.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] j143 commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "j143 (via GitHub)" <gi...@apache.org>.
j143 commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1732522418
Hi @anishsapkota -- let's move this forward, request the reviewer to commit this much by addressing the comments (which seem minor).
> could you please let me know which files I should be looking at, so that I can implement this as well ?
after that you can add further functionality.
Thanks, Janardhan
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SYSTEMDS-3546] Push down image pre-processing to blas/mkl [systemds]
Posted by "j143 (via GitHub)" <gi...@apache.org>.
j143 commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1934179408
marking this PR for next release because there are some concerns pointed by @corepointer
> Our current Intel MKL support ends at version 2019.5 (you noticed the problems with dnn ops not compiling). I would be surprised if all magically worked by just using OneMKL and OneDNN.
>
> We should probably put the native ops testing into a separate test suite and use a Docker container that supports it.
>
> Furthermore, moving to OneAPI would change external behavior and require a major version bump.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1713782833
Regarding the tests, I need to allow them to run since this is your first commit it does not allow them to automatically run.
I will keep an eye on it and run the tests when you commit, if you feel like you are waiting to long just write a comment.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1720942829
@Baunsgaard @j143
Hi,
after some intensive trying using **gitpod** I could conclude following things:
1. **libmkl_rt.so cannot open the file: no such file or directory** this error was because ldconfig could not find the exported LD_LIBRARY_PATH. I could solve this problem by adding a **.conf file** with **/opt/intel/oneapi/mkl/latest/lib/intel64** in **/etc/ld.so.conf.d** directory and reloading **ldconfig.**
2. Next Thanks to @j143 I stumbled across his video about Intel MKL installation on ubuntu. If I would do it just like he mentioned using a **offline package installer** which has CLI based UI and requires user interaction ( _I could not find a way to do this using shell script_), everything would work fine after doing what I mentioned in 1.
If we were to use **apt package installer** instead of offline one, somehow the include folder containing all the necessary header file like "mkl.h" is missing in mkl root folder. I tried using "locate mkl.h" no result.
I could not find the solution to this.
Next option would be to install whole intel-basekit which contains MKL as well. It is unfortunately too large ~13 GB I think and could have led to the not enough space error.
3. Regarding "mkl_dnn.h" .I have mentioned here https://github.com/apache/systemds/pull/1843#issuecomment-1718879255, if we download it unzip it, we have 2 folders in it include and lib, then copy "mkl_dnn.h" and "mkl_dnn_types.h"(second name is something similar) for its include folder to /opt/intel/oneapi/mkl/latest/include (if present) and the libs to /opt/intel/oneapi/mkl/latest/lib/intel64. Using this trick I could compile the cpp files with "mkl_dnn.h"header file as well. **But not recommended it was just an experiment.**
4. Nevertheless, I could run the benchmarks using both mkl and openblas in gitpod. Although I could not install mkl properly in GitHub test docker container.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1720925104
> Hi @Baunsgaard
>
> > System.IO.IOException: No space left on device : '/home/runner/runners/2.309.0/_diag/Worker_20230915-060835-utc.log'
> > Unhandled exception. System.IO.IOException: No space left on device : '/home/runner/runners/2.309.0/_diag/Worker_20230915-060835-utc.log'
>
> https://github.com/apache/systemds/actions/runs/6191936104
well, i guess we need to cleanup some of the resources.
I think we only have 8GB disk space, but i might remember it wrong.
what we need to do is to remove cached files from install of our docker image to reduce its size via `apt-get clean`
docker/testsysds.Dockerfile
Then include MKL and OpenBLAS in the image rather than installing them in the build script of cpp and make sure we do not keep around anything not needed in our install.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1714193172
> > and there it says something like:
>
> yes, I actually adjusted it in CMakeLists.txt, it includes the img lib and the build.sh script should use CMakeList.txt to compile those binaries right ?
>
> CMakeLists.txt set(HEADER_FILES imgUtils.h libmatrixdnn.h libmatrixmult.h systemds.h common.h) set(SOURCE_FILES imgUtils.cpp systemds_img.cpp libmatrixdnn.cpp libmatrixmult.cpp systemds.cpp)
It should yes, but maybe there is something else going wrong.
Perhaps adding another block of code in the GitHub action.
```txt
- name: Build C++
run: |
cd /github/workspace/src/main/cpp
./build.sh
ls lib
```
you can add it after the checkout block.
Maybe that gives you some indications?
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1719034924
libhe-Linux-x86_64.so
libsystemds_mkl-Linux-x86_64.so
libsystemds_mkl-Windows-AMD64.dll
libsystemds_openblas-Linux-x86_64.so
libsystemds_openblas-Windows-AMD64.dll
libsystemds_spoof_cuda-Linux-x86_64.so
libsystemds_spoof_cuda-Windows-AMD64.dll
this was the output of ls /github/workspace/src/main/cpp/lib **libsystemds_mkl-Linux-x86_64.so** seems to be there.
Error: org.apache.sysds.test.functions.nativ.ImgUtilsTest.runTestsWithMKL Time elapsed: 0.001 s <<< ERROR!
**java.lang.UnsatisfiedLinkError: /github/workspace/src/main/cpp/lib/libsystemds_mkl-Linux-x86_64.so:** libmkl_rt.so: cannot open shared object file: **No such file or directory**
at org.apache.sysds.test.functions.nativ.ImgUtilsTest.runTests(ImgUtilsTest.java:497)
at org.apache.sysds.test.functions.nativ.ImgUtilsTest.runTestsWithMKL(ImgUtilsTest.java:35)
it doesn't make sense,
**libsystemds_openblas-Linux-x86_64.so** this one is however loaded without any problem.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1714159860
> We call this script in the beginning of building [ docker/entrypoint.sh](https://github.com/apache/systemds/blob/74be3fcdba8ec1015b860ce4f5a9a6758c306ac9/docker/entrypoint.sh)
>
> can it be that:
>
> ```
> cd /github/workspace/src/main/cpp
> ./build.sh
> cd /github/workspace
> ```
>
> does not build your binaries?
how can I verify that?
there were old binaries in the lib folder. When the git tests run, it should build new ones and replace the old one with it right ?
even if this doesn't happen, the System.load() should load the old binaries that are present in cpp/lib and throw some error that says it could not find the native implementations.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1714170216
> > We call this script in the beginning of building [ docker/entrypoint.sh](https://github.com/apache/systemds/blob/74be3fcdba8ec1015b860ce4f5a9a6758c306ac9/docker/entrypoint.sh)
> > can it be that:
> > ```
> > cd /github/workspace/src/main/cpp
> > ./build.sh
> > cd /github/workspace
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > does not build your binaries?
>
> how can I verify that? there were old binaries in the lib folder. When the git tests run, it should build new ones and replace the old one with it right ?
>
> even if this doesn't happen, the System.load() should load the old binaries that are present in cpp/lib and throw some error that says it could not find the native implementations.
we always build the binaries on the GitHub testing.
you can look up the raw logs :
https://pipelinesghubeus22.actions.githubusercontent.com/99y4vYqbkvSsdnIzLIwwBWoa7RjP8bOodIdS7qcJ1UCIIzC9x5/_apis/pipelines/1/runs/40826/signedlogcontent/30?urlExpires=2023-09-11T15%3A56%3A29.5229959Z&urlSigningMethod=HMACV1&urlSignature=O1AR7JTkHl6vk70WkmpWS8AHt98TXruku%2B9w0t14eJI%3D
and there it says something like:
```
2023-09-11T13:45:53.2908375Z See also "/github/workspace/src/main/cpp/OPENBLAS/CMakeFiles/CMakeOutput.log".
2023-09-11T13:45:53.3071180Z make: *** No rule to make target 'install'. Stop.
2023-09-11T13:45:53.3123270Z -----------------------------------------------------------------------
2023-09-11T13:45:53.3123866Z Check for unexpected dependencies added after code change or new setup:
2023-09-11T13:45:53.3124457Z Non-standard dependencies for libsystemds_mkl-linux-x86_64.so
2023-09-11T13:45:53.3192053Z Non-standard dependencies for libsystemds_openblas-linux-x86_64.so
2023-09-11T13:45:53.3264477Z -----------------------------------------------------------------------
2023-09-11T13:45:53.4061848Z -- The CXX compiler identification is GNU 9.4.0
2023-09-11T13:45:53.4118015Z -- Check for working CXX compiler: /usr/bin/g++
2023-09-11T13:45:53.5109326Z -- Check for working CXX compiler: /usr/bin/g++ -- works
2023-09-11T13:45:53.5126103Z -- Detecting CXX compiler ABI info
2023-09-11T13:45:53.6161549Z -- Detecting CXX compiler ABI info - done
2023-09-11T13:45:53.6300600Z -- Detecting CXX compile features
2023-09-11T13:45:53.6301203Z -- Detecting CXX compile features - done
2023-09-11T13:45:53.6334317Z -- Looking for C++ include pthread.h
2023-09-11T13:45:53.7454940Z -- Looking for C++ include pthread.h - found
2023-09-11T13:45:53.7455614Z -- Performing Test CMAKE_HAVE_LIBC_PTHREAD
2023-09-11T13:45:53.8619331Z -- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Failed
2023-09-11T13:45:53.8621643Z -- Check if compiler accepts -pthread
2023-09-11T13:45:53.9742424Z -- Check if compiler accepts -pthread - yes
2023-09-11T13:45:53.9759399Z -- Found Threads: TRUE
2023-09-11T13:45:53.9771762Z -- Microsoft SEAL -> Version 3.7.0 detected
2023-09-11T13:45:53.9789595Z -- Microsoft SEAL -> Targets available: SEAL::seal_shared
2023-09-11T13:45:53.9790366Z -- Configuring done
2023-09-11T13:45:53.9811958Z -- Generating done
2023-09-11T13:45:53.9817535Z -- Build files have been written to: /github/workspace/src/main/cpp/HE
2023-09-11T13:45:54.0510546Z Scanning dependencies of target he
2023-09-11T13:45:54.0700122Z [ 33%] Building CXX object CMakeFiles/he.dir/libhe.cpp.o
2023-09-11T13:45:58.4275559Z [ 66%] Building CXX object CMakeFiles/he.dir/he.cpp.o
2023-09-11T13:46:02.6181043Z [100%] Linking CXX shared library libhe-Linux-x86_64.so
2023-09-11T13:46:02.7196534Z [100%] Built target he
2023-09-11T13:46:02.7462105Z Install the project...
```
and that indicate to me that the image lib is not compiled.
we need to add it as an element to compile in:
https://github.com/apache/systemds/blob/74be3fcdba8ec1015b860ce4f5a9a6758c306ac9/src/main/cpp/build.sh
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1714471768
/Users/runner/work/_temp/b8ad4948-f59d-4334-ad56-0415e504b59f.sh: line 1: cd: /github/workspace/src/main/cpp: No such file or directory
error during the build after the latest 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.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1722584629
<img width="600" alt="image" src="https://github.com/apache/systemds/assets/68165368/9236727e-a9fe-4b8f-89b7-3bd89c7fe367">
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on a diff in pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on code in PR #1843:
URL: https://github.com/apache/systemds/pull/1843#discussion_r1320614946
##########
src/test/java/org/apache/sysds/test/functions/nativ/ImgUtilsTest.java:
##########
@@ -0,0 +1,412 @@
+/*
+ * 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.test.functions.nativ;
+
+import org.apache.sysds.utils.ImgNativeHelper;
+import org.apache.sysds.utils.NativeHelper;
+import org.junit.Test;
+
+import static org.junit.Assert.assertArrayEquals;
+
+public class ImgUtilsTest {
+
+ /*static {
+ System.loadLibrary("systemds_openblas-Darwin-x86_64");
Review Comment:
I would simply add such a static block inside the ImgNativeHelper.java that loads the correctly compiled file depending on the OS in use, if there is no such file error out.
The reason the Native loader does not work on Mac is simply because we have not yet compiled the files for this.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on a diff in pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on code in PR #1843:
URL: https://github.com/apache/systemds/pull/1843#discussion_r1320630192
##########
src/test/java/org/apache/sysds/test/functions/nativ/ImgUtilsTest.java:
##########
@@ -0,0 +1,412 @@
+/*
+ * 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.test.functions.nativ;
+
+import org.apache.sysds.utils.ImgNativeHelper;
+import org.apache.sysds.utils.NativeHelper;
+import org.junit.Test;
+
+import static org.junit.Assert.assertArrayEquals;
+
+public class ImgUtilsTest {
+
+ /*static {
+ System.loadLibrary("systemds_openblas-Darwin-x86_64");
Review Comment:
Thank you,
while adding the static block, can I assume that the java.library.path is already configured ?
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1713845518
java.lang.UnsatisfiedLinkError: /github/workspace/src/main/cpp/lib/libsystemds_openblas-Linux-x86_64.so: libopenblas.so.0: cannot open shared object file: No such file or directory
I still can't figure it out why it can't find the lib. could you please review the static block from my last 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.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1714118557
<img width="604" alt="peformance_eval" src="https://github.com/apache/systemds/assets/68165368/2f5105d5-36dc-4ce4-bbc4-eab517940105">
here is a result from the performance evaluation on my local machine
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1714580497
> Looking at the other PR, i remembered,
>
> Please also test with uneven images with different number of row and column pixels.
I will try to incorporate this as well. Currently only the square images are supported, at least for rotation.
if I try to rotate an image lets say 4 x 3 by 90 degrees, then the output image has to be 3 x 4 and vice versa? And what if I were to rotate it by 45 degrees, I assume the dimension remains the same but certain part of image will be cut off.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1715709114
I am trying to install the openblas using entrypoint.sh, if it doesn't help I will try to add the install section to docker file itself.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1715765877
> I was going through the failed javaTest, and apparently I am installing the openblas outside of the docker container, I put the install section in build.yml file [c500e11](https://github.com/apache/systemds/commit/c500e1192665f3bea8df0a88135b62857635add2)
>
> should I be putting install OpenBlas section in entrypoint.sh ?
>
> -- Configuring incomplete, errors occurred! See also "/github/workspace/src/main/cpp/INTEL/CMakeFiles/CMakeOutput.log". make: *** No rule to make target 'install'. Stop. -- The CXX compiler identification is GNU 9.4.0 -- Check for working CXX compiler: /usr/bin/g++ -- Check for working CXX compiler: /usr/bin/g++ -- works -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Detecting CXX compile features -- Detecting CXX compile features - done CMake Error at cmake/FindOpenBLAS.cmake:72 (MESSAGE): -- Could not find OpenBLAS lib. Turning OpenBLAS_FOUND off Could not find OpenBLAS Call Stack (most recent call first): CMakeLists.txt:69 (find_package)
You added it in Build. therefore yes it is not in the JavaTest GitHub Action.
I thought you just tried some things out in build, to see what to do to make it work.
I would suggest to modify the actual build.sh script in src/main/cpp
that is running every time even inside the docker image.
in that build.sh script you can detect if the correct requirements are installed and if not install them.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on a diff in pull request #1843: [SYSTEMDS-3546] good start
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on code in PR #1843:
URL: https://github.com/apache/systemds/pull/1843#discussion_r1231986836
##########
src/main/java/org/apache/sysds/utils/NativeHelper.java:
##########
@@ -416,4 +416,7 @@ public static native boolean conv2dSparse(int apos, int alen, int[] aix, double[
// different tradeoffs. In current implementation, we always use GetPrimitiveArrayCritical as it has proven to be
// fastest. We can revisit this decision later and hence I would not recommend removing this method.
private static native void setMaxNumThreads(int numThreads);
+
+ //test interface for native binding
Review Comment:
it is not a test inteface anymore,
please write a full Javadoc definition using:
/** */
##########
src/test/java/org/apache/sysds/test/functions/native/NativeBindingTestWithDgemm.java:
##########
@@ -0,0 +1,74 @@
+import java.util.Random;
+import org.apache.sysds.utils.NativeHelper;
+
+public class NativeBindingTestWithDgemm {
+ static {
+ System.loadLibrary("systemds_mkl-Darwin-x86_64");
+ }
+ // Helper method to flatten a 2D matrix into a 1D array
+ private double[] flattenMatrix(double[][] matrix) {
+ int rows = matrix.length;
+ int columns = matrix[0].length;
+ double[] flattened = new double[rows * columns];
+
+ for (int i = 0; i < rows; i++) {
+ for (int j = 0; j < columns; j++) {
+ flattened[i * columns + j] = matrix[i][j];
+ }
+ }
+
+ return flattened;
+ }
+
+ // Helper method to generate a random matrix
+ private double[][] generateRandomMatrix(int rows, int columns) {
+ Random random = new Random();
+ double[][] matrix = new double[rows][columns];
+
+ for (int i = 0; i < rows; i++) {
+ for (int j = 0; j < columns; j++) {
+ matrix[i][j] = random.nextDouble();
+ }
+ }
+
+ return matrix;
+ }
+
+ // Method to test the dgemm function
+ public void testDgemm() {
+ char transa = 'N';
+ char transb = 'N';
+ int m = 2000;
+ int n = 200;
+ int k = 1000;
+ double alpha = 1.0;
+ double beta = 0.0;
+
+ // Generate random input matrices A, B, and C
+ double[][] A = generateRandomMatrix(m, k);
+ double[][] B = generateRandomMatrix(k, n);
+ double[][] C = new double[m][n];
+
+ // Convert matrices to 1D arrays
+ double[] flatA = flattenMatrix(A);
+ double[] flatB = flattenMatrix(B);
+ double[] flatC = flattenMatrix(C);
+
+ // Call the native dgemm method
+
+ NativeHelper.testNativeBindingWithDgemm(transa, transb, m, n, k, alpha, flatA, k, flatB, n, beta, flatC, n);
+
+ // Print the result matrix C
+ System.out.println("Result Matrix C:");
+ for (int i = 0; i < m; i++) {
+ for (int j = 0; j < n; j++) {
+ System.out.print(flatC[i * n + j] + " ");
+ }
+ System.out.println();
Review Comment:
avoid printing in tests.
A general design for clean tests is to only print once something goes wrong.
you can imagine if we have a million tests that print. then we would not be able to decipher what is wrong or what is right. (and we would use all our time printing in the tests)
##########
src/test/java/org/apache/sysds/test/functions/native/NativeBindingTestWithDgemm.java:
##########
@@ -0,0 +1,74 @@
+import java.util.Random;
Review Comment:
license
##########
src/test/java/org/apache/sysds/test/functions/native/NativeBindingTestWithDgemm.java:
##########
@@ -0,0 +1,74 @@
+import java.util.Random;
+import org.apache.sysds.utils.NativeHelper;
+
+public class NativeBindingTestWithDgemm {
+ static {
+ System.loadLibrary("systemds_mkl-Darwin-x86_64");
+ }
+ // Helper method to flatten a 2D matrix into a 1D array
+ private double[] flattenMatrix(double[][] matrix) {
+ int rows = matrix.length;
+ int columns = matrix[0].length;
+ double[] flattened = new double[rows * columns];
+
+ for (int i = 0; i < rows; i++) {
+ for (int j = 0; j < columns; j++) {
+ flattened[i * columns + j] = matrix[i][j];
+ }
+ }
+
+ return flattened;
+ }
+
+ // Helper method to generate a random matrix
+ private double[][] generateRandomMatrix(int rows, int columns) {
+ Random random = new Random();
+ double[][] matrix = new double[rows][columns];
+
+ for (int i = 0; i < rows; i++) {
+ for (int j = 0; j < columns; j++) {
+ matrix[i][j] = random.nextDouble();
+ }
+ }
+
+ return matrix;
+ }
+
+ // Method to test the dgemm function
+ public void testDgemm() {
+ char transa = 'N';
+ char transb = 'N';
+ int m = 2000;
+ int n = 200;
+ int k = 1000;
+ double alpha = 1.0;
+ double beta = 0.0;
+
+ // Generate random input matrices A, B, and C
+ double[][] A = generateRandomMatrix(m, k);
+ double[][] B = generateRandomMatrix(k, n);
+ double[][] C = new double[m][n];
+
+ // Convert matrices to 1D arrays
+ double[] flatA = flattenMatrix(A);
+ double[] flatB = flattenMatrix(B);
+ double[] flatC = flattenMatrix(C);
+
+ // Call the native dgemm method
+
+ NativeHelper.testNativeBindingWithDgemm(transa, transb, m, n, k, alpha, flatA, k, flatB, n, beta, flatC, n);
+
+ // Print the result matrix C
+ System.out.println("Result Matrix C:");
+ for (int i = 0; i < m; i++) {
+ for (int j = 0; j < n; j++) {
+ System.out.print(flatC[i * n + j] + " ");
+ }
+ System.out.println();
+ }
+ }
+
+ public static void main(String[] args) {
Review Comment:
you can leave this function for tests purposes, but ideally please integrate the test you make into JUnit.
this is as simple as marking methods with \@ Test See other files for examples.
##########
src/main/cpp/libAnish.cpp:
##########
Review Comment:
for the future, if you have general comments i suggest you put them in the general comments, not as a comment on a comment like this. 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.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SYSTEMDS-3546] Push down image pre-processing to blas/mkl [systemds]
Posted by "j143 (via GitHub)" <gi...@apache.org>.
j143 commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1837770453
Hi @Baunsgaard , this PR does seems fine. only minor comments are there. Is it okay to take this in?
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1719324701
maybe the tests run on your fork?
you can disable some of them to not run all the time and then run it there.
note your are limited to 5 at a time so i was trying to avoid this.
to do this,
1. comment out tests that are not relevant for you such as build license python and most of the java tests
3. open the PR on your fork as a PR into your main branch (on your fork.).
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on a diff in pull request #1843: [SYSTEMDS-3546] good start
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on code in PR #1843:
URL: https://github.com/apache/systemds/pull/1843#discussion_r1229479305
##########
src/main/cpp/libAnish.cpp:
##########
Review Comment:
Start with another name of your new cpp file.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] j143 commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "j143 (via GitHub)" <gi...@apache.org>.
j143 commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1718814573
Hi @anishsapkota -- Now, the tests are running.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] j143 commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "j143 (via GitHub)" <gi...@apache.org>.
j143 commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1720749877
Hi @Baunsgaard
> System.IO.IOException: No space left on device : '/home/runner/runners/2.309.0/_diag/Worker_20230915-060835-utc.log'
Unhandled exception. System.IO.IOException: No space left on device : '/home/runner/runners/2.309.0/_diag/Worker_20230915-060835-utc.log'
https://github.com/apache/systemds/actions/runs/6191936104
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1723219006
In the results you have above the time of the `Heavy hitter instructions:` is what i would use to compare. The rest of the time reported is all overhead of Java , I/O and other compilation related elements.
But i would modify the script you use. A example of a measuring script:
```R
res = read($1)
print(sum(res))
for(i in 1:$2) {
res2 = OPERATION(res)
}
print(sum(res2))
```
This reads the file in the first argument and then repeats the operation the number of times specified in the second argument. Because of the way we execute in SystemDS the sum before the loop make sure res is actually read from disk.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1714676884
> > Looking at the other PR, i remembered,
> > Please also test with uneven images with different number of row and column pixels.
>
> I will try to incorporate this as well. Currently only the square images are supported, at least for rotation. if I try to rotate an image lets say 4 x 3 by 90 degrees, then the output image has to be 3 x 4 and vice versa? And what if I were to rotate it by 45 degrees, I assume the dimension remains the same but certain part of image will be cut off.
it is okay if some things are not supported, in such cases we just error out with a reasonable message.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on a diff in pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on code in PR #1843:
URL: https://github.com/apache/systemds/pull/1843#discussion_r1320633495
##########
src/test/java/org/apache/sysds/test/functions/nativ/ImgUtilsTest.java:
##########
@@ -0,0 +1,412 @@
+/*
+ * 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.test.functions.nativ;
+
+import org.apache.sysds.utils.ImgNativeHelper;
+import org.apache.sysds.utils.NativeHelper;
+import org.junit.Test;
+
+import static org.junit.Assert.assertArrayEquals;
+
+public class ImgUtilsTest {
+
+ /*static {
+ System.loadLibrary("systemds_openblas-Darwin-x86_64");
Review Comment:
but still, we need to compile the new implementations in order to include them into the native library. Or should I push my own compiled lib for test ?
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1713790811
> Regarding the tests, I need to allow them to run since this is your first commit it does not allow them to automatically run.
> I will keep an eye on it and run the tests when you commit, if you feel like you are waiting to long just write a comment.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1714176241
> <img alt="peformance_eval" width="604" src="https://user-images.githubusercontent.com/68165368/267070022-2f5105d5-36dc-4ce4-bbc4-eab517940105.png">
>
> here is a result from the performance evaluation on my local machine. I am little bit unsure about the intel mkl, i used this by setting MKL_NUM_THREADS = 4 as I found that it works best for my machine. It was little bit surprising that although I have intel chip, OpenBlas significantly performed better.
This already looks promising,
please run the tests with repeats such that you get the average run time of ~100 runs.
also run it both with a sparse input and a dense input.
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.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1714178323
> and there it says something like:
yes, I actually adjusted it in CMakeLists.txt, it includes the img lib and the build.sh script should use CMakeList.txt to compile those binaries right ?
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1719068915
> libhe-Linux-x86_64.so libsystemds_mkl-Linux-x86_64.so libsystemds_mkl-Windows-AMD64.dll libsystemds_openblas-Linux-x86_64.so libsystemds_openblas-Windows-AMD64.dll libsystemds_spoof_cuda-Linux-x86_64.so libsystemds_spoof_cuda-Windows-AMD64.dll
>
> this was the output of ls /github/workspace/src/main/cpp/lib **libsystemds_mkl-Linux-x86_64.so** seems to be there.
>
> Error: org.apache.sysds.test.functions.nativ.ImgUtilsTest.runTestsWithMKL Time elapsed: 0.001 s <<< ERROR! **java.lang.UnsatisfiedLinkError: /github/workspace/src/main/cpp/lib/libsystemds_mkl-Linux-x86_64.so:** libmkl_rt.so: cannot open shared object file: **No such file or directory** at org.apache.sysds.test.functions.nativ.ImgUtilsTest.runTests(ImgUtilsTest.java:497) at org.apache.sysds.test.functions.nativ.ImgUtilsTest.runTestsWithMKL(ImgUtilsTest.java:35)
>
> it doesn't make sense,
>
> **libsystemds_openblas-Linux-x86_64.so** this one is however loaded without any problem from same path
i am unfortunately unaware what needs to be loaded when, there must be some sequence or elements missing. this libmkl_rt.so is what is says is missing now, not your build mkl file.
About unrelated tests to native, none of them should be effected by your current changes, so if they fail it is okay, we can rerun them to see if it is a systematic error. On a side node i suspect that they run the PR actions on some other hardware that sometimes is more prone to errors, but i have no way of confirming 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.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1717244325
Hi @Baunsgaard
I think there is something wrong with my last commit, not even a single javaTest ran. is it possible to intervene this and run the latest commit, the one with "OneApi"?
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1719524937
Thank you @Baunsgaard @j143 ,
Now I can run the tests on my own. :)
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1717851113
Hi @Baunsgaard ,
could you please review the javaTest that are failing? They are not the part of my implementations and seem to somehow fail, I could not figure it out why
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1718186728
Hi @Baunsgaard,
I have a situation here. I was able to successfully install INTEL oneApi Basekit which includes newer Intel MKL lib earlier and run the setvars.sh script to set the env variables. But the installation conflicts with the earlier implementations made with older INTEL MKL. It says "mkl_dnn.h" not found during build.
The older version of intel from 2020 also does not includes "mkl_dnn.h" either.
Please guide me through this.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1719220189
**-- Found MKL (include: /opt/intel/oneapi/mkl/2023.2.0/include, lib: "/opt/intel/oneapi/mkl/2023.2.0/lib/intel64/libmkl_rt.so"**
it finds the shared object during the build. But I can't figure it out why it's causing problem during loading. As I don't have a linux system I really cannot further investigate it and solely rely on these tests which is very inefficient. I tried using the UNI's Linux terminals, but they do not allow any package installations from the internet. My laptop would crash while using Virtual box with Ubuntu. :(
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1722584605
<img width="793" alt="image" src="https://github.com/apache/systemds/assets/68165368/c656f62c-1f6c-44db-acfc-c4a65135881b">
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] j143 commented on a diff in pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "j143 (via GitHub)" <gi...@apache.org>.
j143 commented on code in PR #1843:
URL: https://github.com/apache/systemds/pull/1843#discussion_r1335146600
##########
src/test/java/org/apache/sysds/test/functions/nativ/ImgUtilsTest.java:
##########
@@ -0,0 +1,521 @@
+/*
+ * 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.test.functions.nativ;
+
+import org.apache.sysds.utils.ImgNativeHelper;
+import org.junit.Test;
+import static org.junit.Assert.assertArrayEquals;
+
+public class ImgUtilsTest {
+
+ @Test
+ public void runTestsWithOpenBlas() {
+ runTests("openblas");
+ }
+
+ //TODO configure intel mkl in test docker container to run this test
+ public void runTestsWithMKL() {
+ runTests("mkl");
+ }
+
+
+
+ public static void testImageRotation90And45(ImgNativeHelper imgNativeHelper) {
+
+ // Input image dimensions
+ int rows = 3;
+ int cols = 3;
+ // Input image
+ double[] img_in = {
+ 1,2,3,
+ 4,5,6,
+ 7,8,9
+ };
+ // Rotation angle in radians
+ double radians = Math.PI / 2;
+ // Fill value for the output image
+ double fill_value = 0.0;
+ // Expected output image
+ double[] expected_img_out_90 = {
+ 3,6,9,
+ 2,5,8,
+ 1,4,7
+ };
+
+ double[] expected_img_out_45 = {
+ 2,3,6,
+ 1,5,9,
+ 4,7,8
+ };
+
+ // Create the output image array
+ double[] img_out = new double[rows * cols];
+ imgNativeHelper.imageRotate(img_in, rows, cols, radians, fill_value, img_out);
+ assertArrayEquals(expected_img_out_90, img_out, 0.0001);
+ //rotate by 45
+ imgNativeHelper.imageRotate(img_in, rows, cols, radians/2, fill_value, img_out);
+ assertArrayEquals(expected_img_out_45, img_out, 0.0001);
+ }
+
+
+
+ public static void testImageRotation90And45_4x4(ImgNativeHelper imgNativeHelper) {
+
+ // Input image dimensions
+ int rows = 4;
+ int cols = 4;
+ // Input image
+ double[] img_in = {
+ 1,2,3,5,
+ 4,5,6,5,
+ 7,8,9,5,
+ 5,5,5,5,
+ };
+ // Rotation angle in radians
+ double radians = Math.PI / 2;
+ // Fill value for the output image
+ double fill_value = 0.0;
+ // Expected output image
+ double[] expected_img_out_90 = {
+ 5,5,5,5,
+ 3,6,9,5,
+ 2,5,8,5,
+ 1,4,7,5
+ };
+
+
+ double[] expected_img_out_45 = {
+ 0,3,5,0,
+ 2,5,6,5,
+ 4,5,8,5,
+ 0,7,5,0
+ };
+
+ // Create the output image array
+ double[] img_out = new double[rows * cols];
+ imgNativeHelper.imageRotate(img_in, rows, cols, radians, fill_value, img_out);
+ assertArrayEquals(expected_img_out_90, img_out, 0.0001);
+ //rotate by 45
+ imgNativeHelper.imageRotate(img_in, rows, cols, radians/2, fill_value, img_out);
+ assertArrayEquals(expected_img_out_45, img_out, 0.0001);
+ }
+
+
+ public static void testImageRotation45(ImgNativeHelper imgNativeHelper) {
+ // Input image dimensions
+ int rows = 6;
+ int cols = 6;
+
+ // Input image
+ double[] img_in = {
+ 1.0, 2.0, 3.0, 4.0,5.0,6.0,
+ 5.0, 6.0, 7.0, 8.0,9.0,10.0,
+ 9.0, 10.0, 11.0, 12.0,13.0,14.0,
+ 13.0, 14.0, 15.0, 16.0,17.0,18.0,
+ 17.0, 18.0, 19.0, 20.0, 21.0, 22.0,
+ 21.0,22.0,23.0,24.0,25.0,26.0
+ };
+
+ // Rotation angle in radians
+ double radians = Math.PI / 4;
+
+ // Fill value for the output image
+ double fill_value = 0.0;
+
+ // Expected output image
+ double[] expected_img_out = {
+ 0, 4, 5, 10 ,14, 0,
+ 3, 4, 8 ,13 ,18 ,18,
+ 2, 7, 12 ,16 ,17 ,22,
+ 5, 10, 15 ,16, 20 ,25,
+ 9, 13, 14, 19, 24 ,24,
+ 0, 13, 17 ,22, 23 ,0
+ };
+
+ // Create the output image array
+ double[] img_out = new double[rows * cols];
+
+ // Rotate the image
+ imgNativeHelper.imageRotate(img_in, rows, cols, radians, fill_value, img_out);
+
+ // Compare the output image with the expected image
+ assertArrayEquals(expected_img_out, img_out, 0.0001);
+ }
+
+
+ public static void testImageRotation180(ImgNativeHelper imgNativeHelper) {
+ // Input image dimensions
+ int rows = 4;
+ int cols = 4;
+
+ // Input image
+ double[] img_in = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 6.0, 7.0, 8.0,
+ 9.0, 10.0, 11.0, 12.0,
+ 13.0, 14.0, 15.0, 16.0
+ };
+
+ // Rotation angle in radians
+ double radians = Math.PI;
+
+ // Fill value for the output image
+ double fill_value = 0.0;
+
+ // Expected output image
+ double[] expected_img_out = {
+ 16.0, 15.0, 14.0, 13.0,
+ 12.0, 11.0, 10.0, 9.0,
+ 8.0, 7.0, 6.0, 5.0,
+ 4.0, 3.0, 2.0, 1.0
+ };
+
+ // Create the output image array
+ double[] img_out = new double[rows * cols];
+
+ // Rotate the image
+ imgNativeHelper.imageRotate(img_in, rows, cols, radians, fill_value, img_out);
+
+ // Compare the output image with the expected image
+ assertArrayEquals(expected_img_out, img_out, 0.0001);
+ }
+
+
+ public static void testCutoutImage(ImgNativeHelper imgNativeHelper) {
+ // Example input 2D matrix
+ double[] img_in = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 6.0, 7.0, 8.0,
+ 9.0, 10.0, 11.0, 12.0,
+ 13.0, 14.0, 15.0, 16.0
+ };
+
+ int rows = 4;
+ int cols = 4;
+ int x = 2;
+ int y = 2;
+ int width = 3;
+ int height = 3;
+ double fill_value = 0.0;
+
+ // Perform image cutout using JNI
+ double[] img_out = imgNativeHelper.imageCutout(img_in, rows, cols, x, y, width, height, fill_value);
+
+ // Expected output image after cutout
+ double[] expectedOutput = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 0.0, 0.0, 0.0,
+ 9.0, 0.0, 0.0, 0.0,
+ 13.0, 0.0, 0.0, 0.0
+ };
+
+ // Check if the output image matches the expected output
+ assertArrayEquals(expectedOutput, img_out,0.0001);
+ }
+
+
+ public static void testCutoutImageNonSquare(ImgNativeHelper imgNativeHelper) {
+ // Example input 2D matrix
+ double[] img_in = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 6.0, 7.0, 8.0,
+ 9.0, 10.0, 11.0, 12.0
+ };
+
+ int rows = 3;
+ int cols = 4;
+ int x = 1;
+ int y = 1;
+ int width = 3;
+ int height = 2;
+ double fill_value = 0.0;
+
+ // Perform image cutout using JNI
+ double[] img_out = imgNativeHelper.imageCutout(img_in, rows, cols, x, y, width, height, fill_value);
+
+ // Expected output image after cutout
+ double[] expectedOutput = {
+ 0.0, 0.0, 0.0, 4.0,
+ 0.0, 0.0, 0.0, 8.0,
+ 9.0, 10.0, 11.0, 12.0
+ };
+
+ // Check if the output image matches the expected output
+ assertArrayEquals(expectedOutput, img_out, 0.0001);
+ }
+
+
+ public static void testImageCutoutInvalidCutout(ImgNativeHelper imgNativeHelper) {
+ double[] img_in = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 6.0, 7.0, 8.0,
+ 9.0, 10.0, 11.0, 12.0,
+ 13.0, 14.0, 15.0, 16.0
+ };
+
+ int rows = 4;
+ int cols = 4;
+ int x = 3;
+ int y = 3;
+ int width = -2;
+ int height = 0;
+ double fill_value = 0.0;
+
+ double[] expectedOutput = img_in; // Expect no change since the cutout is invalid
+
+ double[] img_out = imgNativeHelper.imageCutout(img_in, rows, cols, x, y, width, height, fill_value);
+ assertArrayEquals(expectedOutput, img_out,0.0001);
+ }
+
+
+ public static void testImageCutoutNoCutout(ImgNativeHelper imgNativeHelper) {
+ double[] img_in = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 6.0, 7.0, 8.0,
+ 9.0, 10.0, 11.0, 12.0,
+ 13.0, 14.0, 15.0, 16.0
+ };
+
+ int rows = 4;
+ int cols = 4;
+ int x = 3;
+ int y = 3;
+ int width = 1;
+ int height = 1;
+ double fill_value = 0.0;
+
+ double[] expectedOutput = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 6.0, 7.0, 8.0,
+ 9.0, 10.0, 0.0, 12.0,
+ 13.0, 14.0, 15.0, 16.0
+ };
+
+ double[] img_out = imgNativeHelper.imageCutout(img_in, rows, cols, x, y, width, height, fill_value);
+ assertArrayEquals(expectedOutput, img_out,0.0001);
+ }
+
+
+ public static void testImageCropValidCrop(ImgNativeHelper imgNativeHelper) {
+ double[] img_in = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 6.0, 7.0, 8.0,
+ 9.0, 10.0, 11.0, 12.0,
+ 13.0, 14.0, 15.0, 16.0
+ };
+
+ int orig_w = 4;
+ int orig_h = 4;
+ int w = 2;
+ int h = 2;
+ int x_offset = 1;
+ int y_offset = 1;
+
+ double[] expectedOutput = {
+ 6.0, 7.0,
+ 10.0, 11.0
+ };
+
+ double[] img_out = imgNativeHelper.cropImage(img_in, orig_w, orig_h, w, h, x_offset, y_offset);
+
+ assertArrayEquals(expectedOutput, img_out,0.0001); }
+
+
+ public static void testImageCropInvalidCrop(ImgNativeHelper imgNativeHelper) {
+ double[] img_in = {
+ 1.0, 2.0, 3.0, 4.0,
+ 5.0, 6.0, 7.0, 8.0,
+ 9.0, 10.0, 11.0, 12.0,
+ 13.0, 14.0, 15.0, 16.0
+ };
+
+ int orig_w = 4;
+ int orig_h = 4;
+ int w = 5;
+ int h = 5;
+ int x_offset = 1;
+ int y_offset = 1;
+
+ double[] expectedOutput = img_in; // Expect no change since the crop is invalid
+
+ double[] img_out = imgNativeHelper.cropImage(img_in, orig_w, orig_h, w, h, x_offset, y_offset);
+ assertArrayEquals(expectedOutput, img_out,0.0001);
+ }
+
+
+ public void testImgTranslate(ImgNativeHelper imgNativeHelper) {
+ int in_w = 5;
+ int in_h = 5;
+ int out_w = 7;
+ int out_h = 7;
+ double fill_value = 0.0;
+
+ double[] img_in = new double[in_w * in_h];
+ for (int i = 0; i < in_w * in_h; ++i) {
+ img_in[i] = i + 1; // Filling input image with sequential values
+ }
+
+ double[] img_out = new double[out_w * out_h];
+
+ double offset_x = 1.5;
+ double offset_y = 1.5;
+
+ imgNativeHelper.imgTranslate(img_in, offset_x, offset_y, in_w, in_h, out_w, out_h, fill_value, img_out);
+
+ // Expected output based on the given offsets and fill value
+ double[] expectedOutput = {
+ 0,0,0,0,0,0,0,
+ 0,0,0,0,0,0,0,
+ 0,0,1,2,3,4,5,
+ 0,0,6,7,8,9,10,
+ 0,0,11,12,13,14,15,
+ 0,0,16,17,18,19,20,
+ 0,0,21,22,23,24,25
+ };
+
+ assertArrayEquals(expectedOutput, img_out, 1e-9); // Compare arrays with a small epsilon
+ }
+
+
+ public static void testImgTranslateNegativeOffsets(ImgNativeHelper imgNativeHelper) {
+ int in_w = 5;
+ int in_h = 5;
+ int out_w = 6;
+ int out_h = 6;
+ double fill_value = 0.0;
+
+ double[] img_in = new double[in_w * in_h];
+ for (int i = 0; i < in_w * in_h; ++i) {
+ img_in[i] = i + 1; // Filling input image with sequential values
+ }
+
+ double[] img_out = new double[out_w * out_h];
+
+ double offset_x = -0.5; // Negative offset in X direction
+ double offset_y = -0.5; // Negative offset in Y direction
+
+ imgNativeHelper.imgTranslate(img_in, offset_x, offset_y, in_w, in_h, out_w, out_h, fill_value, img_out);
+
+ // Expected output based on the given offsets and fill value
+ double[] expectedOutput = {
+ 1,2,3,4,5,0,
+ 6,7,8,9,10,0,
+ 11,12,13,14,15,0,
+ 16,17,18,19,20,0,
+ 21,22,23,24,25,0,
+ 0,0,0,0,0,0,
+ };
+
+ assertArrayEquals(expectedOutput, img_out, 1e-9); // Compare arrays with a small epsilon
+ }
+
+
+ public static void testImgTranslatePositiveAndNegativeOffsets(ImgNativeHelper imgNativeHelper) {
+ int in_w = 5;
+ int in_h = 5;
+ int out_w = 6;
+ int out_h = 6;
+ double fill_value = 0.0;
+
+ double[] img_in = new double[in_w * in_h];
+ for (int i = 0; i < in_w * in_h; ++i) {
+ img_in[i] = i + 1; // Filling input image with sequential values
+ }
+
+ double[] img_out = new double[out_w * out_h];
+
+ double offset_x = -0.5; // Negative offset in X direction
+ double offset_y = 0.5; // Negative offset in Y direction
+
+ imgNativeHelper.imgTranslate(img_in, offset_x, offset_y, in_w, in_h, out_w, out_h, fill_value, img_out);
+
+ // Expected output based on the given offsets and fill value
+ double[] expectedOutput = {
+ 0,0,0,0,0,0,
+ 1,2,3,4,5,0,
+ 6,7,8,9,10,0,
+ 11,12,13,14,15,0,
+ 16,17,18,19,20,0,
+ 21,22,23,24,25,0,
+ };
+
+ assertArrayEquals(expectedOutput, img_out, 1e-9); // Compare arrays with a small epsilon
+ }
+
+
+ public static void testImgTranslatePositiveAndNegativeOffsets2(ImgNativeHelper imgNativeHelper) {
+ int in_w = 5;
+ int in_h = 5;
+ int out_w = 6;
+ int out_h = 6;
+ double fill_value = 0.0;
+
+ double[] img_in = new double[in_w * in_h];
+ for (int i = 0; i < in_w * in_h; ++i) {
+ img_in[i] = i + 1; // Filling input image with sequential values
+ }
+
+ double[] img_out = new double[out_w * out_h];
+
+ double offset_x = 0.5; // Negative offset in X direction
+ double offset_y = -0.5; // Negative offset in Y direction
+
+ imgNativeHelper.imgTranslate(img_in, offset_x, offset_y, in_w, in_h, out_w, out_h, fill_value, img_out);
+
+ // Expected output based on the given offsets and fill value
+ double[] expectedOutput = {
+ 0,1,2,3,4,5,
+ 0,6,7,8,9,10,
+ 0,11,12,13,14,15,
+ 0,16,17,18,19,20,
+ 0,21,22,23,24,25,
+ 0,0,0,0,0,0,
+ };
+
+ assertArrayEquals(expectedOutput, img_out, 1e-9); // Compare arrays with a small epsilon
+ }
+
+ public void runTests(String blasType) {
Review Comment:
Hi @anishsapkota , there are a plenty of tests in the neighboring folders you can take inspiration from!
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1717375389
@Baunsgaard could you please approved the workflow again, previous installation got stuck as it was waiting for user interaction.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1715238266
Hi @Baunsgaard ,
could you please let the tests run, once you are online. Thank you.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1713738670
Hi @Baunsgaard ,
I am waiting for the tests to run, so that we can merge.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1714460315
> Performance Evaluation averaged over 100 runs: <img alt="Screenshot 2023-09-11 at 19 59 44" width="518" src="https://user-images.githubusercontent.com/68165368/267107362-797db0d7-1c44-4475-8d19-94efa751b152.png">
>
> I observed that the first run takes significantly more time in comparison to later runs, although the input matrices are completely randomised.
great!
There is an option of removing outliers what I typically do is remove the 5% best and worst performing runs and then do the average of the remaining. Also good practice is to calculate std on the runs and mark it in the graphs with +-, this is similarly done without the 5%.
Also for more clarification of where time is spend I would love it if you run SystemDS with the -stats flag.
this can indicate where we are using time in the DML implementation
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1713928055
We call this script in the beginning of building
[
docker/entrypoint.sh](https://github.com/apache/systemds/blob/74be3fcdba8ec1015b860ce4f5a9a6758c306ac9/docker/entrypoint.sh)
can it be that:
```
cd /github/workspace/src/main/cpp
./build.sh
cd /github/workspace
```
does not build your binaries?
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1714485932
> /Users/runner/work/_temp/b8ad4948-f59d-4334-ad56-0415e504b59f.sh: line 1: cd: /github/workspace/src/main/cpp: No such file or directory
>
> error during the build after the latest commit
okay, then since it does not work... lets do relative paths like in:
```
- name: Build Python Package
run: |
cd src/main/python
python create_python_dist.py
```
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1715932977
Hi @Baunsgaard
could you please approve the workflow ? Thank you
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] anishsapkota commented on pull request #1843: [SYSTEMDS-3546] Push down image pre-processing to blas/mkl
Posted by "anishsapkota (via GitHub)" <gi...@apache.org>.
anishsapkota commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1722584476
> The fairest comparison would be to integrate the native instruction as a DML supported command. It does not take long to make. But requires you to know where to do it.
could you please let me know which files I should be looking at, so that I can implement this as well ?
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SYSTEMDS-3546] Push down image pre-processing to blas/mkl [systemds]
Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #1843:
URL: https://github.com/apache/systemds/pull/1843#issuecomment-1838808173
> Hi @Baunsgaard , this PR does seems fine. only minor comments are there. Is it okay to take this in?
Hi @j143 , it can be taken in but we need to be careful, and go through the code, since some of the changes (for instance GitHub actions) contain changes that should not be made.
--
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-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org