You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@systemds.apache.org by GitBox <gi...@apache.org> on 2021/12/10 16:13:56 UTC
[GitHub] [systemds] DavidSandruTU opened a new pull request #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
DavidSandruTU opened a new pull request #1481:
URL: https://github.com/apache/systemds/pull/1481
Couple of notes, let me know what you think:
- I've added an extra dot to data generation scripts, to go back a directory, if the command is "systemds". This is done to compensate the different folder structure of the perftests when ran locally, instead of on a cluster.
- I'm assuming the federated tests are to be ran locally only, not on a cluster.
- The stratified statistics benchmark needs a heap size of at least 10 GB. This setting is exported only when it's ran locally via "export SYSTEMDS_STANDALONE_OPTS="-Xmx10g -Xms10g -Xmn2000m"
- The K-means built-in function does a predict implicitly, meaning the benchmark predicts twice, as of now.
- Could not get the decision tree training built-in function to run.
- It does not stop when ran with logistic regression data.
- There are scripts to generate decision tree data, but these can not be ran either as the built-in transform function does not exist anymore, apparently.
- Could also not get the old decision tree and random forest algorithms running without looking deeper into it. (null pointer exception)
--
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] DavidSandruTU commented on a change in pull request #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
Posted by GitBox <gi...@apache.org>.
DavidSandruTU commented on a change in pull request #1481:
URL: https://github.com/apache/systemds/pull/1481#discussion_r774806125
##########
File path: scripts/perftest/genALSData.sh
##########
@@ -21,30 +21,31 @@
#-------------------------------------------------------------
CMD=$1
-DATADIR=$2
+DATADIR=$2/als
-FORMAT="binary" # can be csv, mm, text, binary
+FORMAT="text" # can be csv, mm, text, binary
DENSE_SP=0.9
SPARSE_SP=0.01
-BASEPATH=$(dirname $0)
+EXTRADOT=""
+if [ "${CMD}" == "systemds" ]; then EXTRADOT="." ; fi
#generate XS scenarios (80MB)
-${CMD} -f ${BASEPATH}/../datagen/genRandData4ALS.dml --nvargs X=${DATADIR}/X10k_1k_dense rows=10000 cols=1000 rank=10 nnz=`echo "scale=0; 10000 * 1000 * $DENSE_SP" | bc` sigma=0.01 fmt=$FORMAT
-${CMD} -f ${BASEPATH}/../datagen/genRandData4ALS.dml --nvargs X=${DATADIR}/X10k_1k_sparse rows=10000 cols=1000 rank=10 nnz=`echo "scale=0; 10000 * 1000 * $SPARSE_SP" | bc` sigma=0.01 fmt=$FORMAT
+${CMD} -f ${EXTRADOT}./datagen/genRandData4ALS.dml --nvargs X=${DATADIR}/X10k_1k_dense rows=10000 cols=1000 rank=10 nnz=`echo "scale=0; 10000 * 1000 * $DENSE_SP" | bc` sigma=0.01 fmt=$FORMAT
Review comment:
Do you mean one folder further in, in the fed folder?
I've changed that script to be used by the general ALS benchmarks, as it was previously used by the federated ALS benchmark. The federated benchmarks now have a new genALS_FedData.sh in the fed folder.
--
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] mboehm7 removed a comment on pull request #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
Posted by GitBox <gi...@apache.org>.
mboehm7 removed a comment on pull request #1481:
URL: https://github.com/apache/systemds/pull/1481#issuecomment-1003424325
For PCA with many columns we have PPCA, which runs this scenario as follows:
```
Total elapsed time: 6.880 sec.
Total compilation time: 1.586 sec.
Total execution time: 5.293 sec.
Number of compiled Spark inst: 0.
Number of executed Spark inst: 0.
Cache hits (Mem/Li/WB/FS/HDFS): 21/0/0/0/0.
Cache writes (Li/WB/FS/HDFS): 3/6/0/2.
Cache times (ACQr/m, RLS, EXP): 0.000/0.000/0.004/4.511 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.
Total JIT compile time: 3.13 sec.
Total JVM GC count: 4.
Total JVM GC time: 0.138 sec.
Heavy hitter instructions:
# Instruction Time(s) Count
1 write 4.511 2
2 rand 0.324 7
3 + 0.256 12
4 leftIndex 0.161 3
5 / 0.025 1
6 n+ 0.017 1
7 * 0.015 5
8 rmvar 0.007 15
9 ceil 0.004 2
10 createvar 0.001 21
```
Similar to lm and als, we should dispatch to the appropriate pca algorithm based on heuristics. 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] Baunsgaard commented on pull request #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on pull request #1481:
URL: https://github.com/apache/systemds/pull/1481#issuecomment-1017748493
Thanks for the PR :rocket: !
It is now merged, while merging i have changed the logging slightly to reduce the output, and i have changed the default to only run the smallest version of the benchmark with 80MB input
--
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 #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on pull request #1481:
URL: https://github.com/apache/systemds/pull/1481#issuecomment-1002201875
Hi @DavidSandruTU !
Thanks for the contribution, :1st_place_medal: ,
i will take it over from here addressing the remaining points and merge it shortly.
to cover your answers and questions:
> Following tests were performed on an old intel i5-6500 processor
>
> Most algorithms execute in similar time of a couple of seconds, but here are some outliers:
>
> **StratifiedStatistics on temp/stratstats/A_10k/data: 39.069242381**
this sound reasonable.
> PCA is especially heavy hitting it seems, but in line with the algorithm I think(?):
>
> **PCA on temp/dimensionreduction/pcaData5k_2k_dense: 285.998737102**
Yes, PCA performance gets bad once the input have many columns, it has two bottle necks, first TSMM that is worse with many columns and potentially rows then eigen, that also is bad with many columns.
> ALS seems to generally be somewhat heavy hitting (~20-30 seconds), but a more interesting outlier is the direct solve algorithm on sparse data. It should perform better in theory, with optimizations, than the same algorithm with dense data.
>
> **ALS-DS algorithm on temp/als/X10k_1k_sparse: 132.269487990**
Great observation, sometimes the optimizations from sparse is not beneficial for different reasons, but i will look into this one since the difference is in 4-7x.
> > 1. Make all scripts executable from the same directory starting point.
> > i suggest, 'scripts/perftest' and write an error if you are not in a directory named that.
>
> Alright, here I want to make sure I've understood everything correctly. If I remember right, a single perftest folder, with all necessary logic, is copied onto the cluster and executed. I think this is why Matthias changed the data generation scripts, in a previous commit, to execute the DMLs in "./datagen/...", instead of "../datagen/...".
>
> What should I do here? Should I assume the datagen scripts are in the perftest folder or outside of it? Want me to add a copy of the datagen scripts to the perftest folder? Also, I'd then make sure the scripts are in the current directory 'perftest', as I think there would be no 'scripts' parent directory. Am I right in that assumption?
The structure of the folders should be assumed to be the same as in the repository, not copied in different ways.
> > 2. Suggestion, make all the code available tor scaling experiments in all cases, but add an argument that specify max memory. For instance 'genClusteringData.sh 80' indicating 80MB max, and make this consistent across scripts.
>
> white_check_mark Please check my code and let me know if you like my approach. E.g. genBinomialData.sh and runAllBinomial.sh
Great!
> > 3. Go through your changes and change all the &>> to &> and >> to >, I am a bit confused about the '&' part of it, since i don't know what it does in context to '>'. If you can explain it it would be nice, otherwise remove the '&' from the piping to files. (this comment is unrelated to adding '&' at the end of the lines, since i know this parallelize the instruction)
>
> white_check_mark I think the "&" is so that error output is also put into the file, not only the standard out. I kept it that way from the original scripts, but never confirmed it myself. Most outputs are changed to now overwrite, not append, except for some special cases:
>
> * The federated algorithm often accesses a file multiple times, e.g. once when creating and once when killing workers. Here I append instead of overwrite.
>
> * Similar with results/times.txt, where all algorithms append their performance times.
Good!
> > 5. Make some of the smallest data generation run in parallel, this is simple to add with a & on the end of the line in sh, and then add a 'wait' in the end of the scripts.
>
> white_check_mark Some scripts have commands that split data into test and training data. These depend on the previous data generation command. I've added multiple waits with PIDs for that, let me know if you're happy with that. E.g. genBinomialData.sh
This is great!
> > 6. Run all... should run all, don't comment anything out. but add an argument to stop execution based on the max Memory argument
>
> Do you want me to uncomment the matrix multiply and transform benchmark too, or was that just meant in context of the max memory argument?
yes, i want to assume that everything is run since the intention is to get performance benchmarks, to find regressions then running everything to get numbers is the default action and therefore what the scripts should reflect.
> I've moved/copied some bash logic relating to the federated tests into the fed folder, as I've written new performance tests for als-predict and als-DS. These new perftests were originally not part of the federated tests, which is why I added the "original" versions to the fed folder, to be executed by the fed tests alone.
Good!
> I've made a file "env-variables", which is sourced once in runAll.sh. The problem here is, if the federated test is ran with that config (from runAll.sh), then each worker then has that heap size (e.g. 10GB), which crashes the test in my case. I've added that "export" commented to the file.
This is okay, we probably should modify the federated performance suite to use different memory budgets for the workers and controller anyway but this is out of scope for this commit.
> > > ```
> > > * It does not stop when ran with logistic regression data.
> > > ```
> >
> >
> > hmm, this might be a bug, please if you want to, report it as such in JIRA with a way to reproduce it.
> > https://issues.apache.org/jira/secure/Dashboard.jspa?selectPageId=12335852
>
> Will do.
Thanks for the issue.
--
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] DavidSandruTU edited a comment on pull request #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
Posted by GitBox <gi...@apache.org>.
DavidSandruTU edited a comment on pull request #1481:
URL: https://github.com/apache/systemds/pull/1481#issuecomment-1003033522
Changes are up.
I also have a commit ready that reproduces the indefinitely running decision tree, with scripts in the perftest/todo directory. I'd reference them in the JIRA.
May I push it?
--
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 closed pull request #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
Posted by GitBox <gi...@apache.org>.
Baunsgaard closed pull request #1481:
URL: https://github.com/apache/systemds/pull/1481
--
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] DavidSandruTU commented on pull request #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
Posted by GitBox <gi...@apache.org>.
DavidSandruTU commented on pull request #1481:
URL: https://github.com/apache/systemds/pull/1481#issuecomment-1000563066
> Hi, @DavidSandruTU !
Thanks for the feedback, @Baunsgaard !
> This is a good start, i have only minor things, but each take a bit of time. Just reach out if there is questions, also please try to run it once you have the limit argument just with the smallest size of each data, and post some of the numbers in the comments :), especially if you find some of them interesting or weird.
Following tests were performed on an old intel i5-6500 processor
Most algorithms execute in similar time of a couple of seconds, but here are some outliers:
StratifiedStatistics on temp/stratstats/A_10k/data: 39.069242381
PCA is especially heavy hitting it seems, but in line with the algorithm I think(?):
PCA on temp/dimensionreduction/pcaData5k_2k_dense: 285.998737102
ALS seems to generally be somewhat heavy hitting (~20-30 seconds), but a more interesting outlier is the direct solve algorithm on sparse data. It should perform better in theory, with optimizations, than the same algorithm with dense data.
ALS-DS algorithm on temp/als/X10k_1k_sparse: 132.269487990
> 1. Make all scripts executable from the same directory starting point.
> i suggest, 'scripts/perftest' and write an error if you are not in a directory named that.
Alright, here I want to make sure I've understood everything correctly. If I remember right, a single perftest folder, with all necessary logic, is copied onto the cluster and executed. I think this is why Matthias changed the data generation scripts, in a previous commit, to execute the DMLs in "./datagen/...", instead of "../datagen/...".
What should I do here? Should I assume the datagen scripts are in the perftest folder or outside of it? Want me to add a copy of the datagen scripts to the perftest folder?
Also, I'd then make sure the scripts are in the current directory 'perftest', as I think there would be no 'scripts' parent directory. Am I right in that assumption?
> 2. Suggestion, make all the code available tor scaling experiments in all cases, but add an argument that specify max memory. For instance 'genClusteringData.sh 80' indicating 80MB max, and make this consistent across scripts.
✅
Please check my code and let me know if you like my approach. E.g. genBinomialData.sh and runAllBinomial.sh
> 3. Go through your changes and change all the &>> to &> and >> to >, I am a bit confused about the '&' part of it, since i don't know what it does in context to '>'. If you can explain it it would be nice, otherwise remove the '&' from the piping to files. (this comment is unrelated to adding '&' at the end of the lines, since i know this parallelize the instruction)
✅
I think the "&" is so that error output is also put into the file, not only the standard out. I kept it that way from the original scripts, but never confirmed it myself.
Most outputs are changed to now overwrite, not append, except for some special cases:
- The federated algorithm often accesses a file multiple times, e.g. once when creating and once when killing workers. Here I append instead of overwrite.
- Similar with results/times.txt, where all algorithms append their performance times.
> 4. Don't change permissions on log4j files and DML files.
> You have done so on:
>
> * scripts/perftest/conf/log4j.properties
> * scripts/perftest/fed/data/splitAndMakeFederated.dml
> * scripts/perftest/scripts/GLM-predict.dml
> * scripts/perftest/scripts/alsCG.dml
✅
Today I learned, Git commits permissions.
> 5. Make some of the smallest data generation run in parallel, this is simple to add with a & on the end of the line in sh, and then add a 'wait' in the end of the scripts.
✅
Some scripts have commands that split data into test and training data. These depend on the previous data generation command. I've added multiple waits with PIDs for that, let me know if you're happy with that. E.g. genBinomialData.sh
> 6. Run all... should run all, don't comment anything out. but add an argument to stop execution based on the max Memory argument
Do you want me to uncomment the matrix multiply and transform benchmark too, or was that just meant in context of the max memory argument?
> 7. Make all the loops go though all datasets, subject to the memory constraints, example 'scripts/perftest/runAllMultinominal.sh'
✅
This should be related to point 2. and 6., and is implemented.
> 8. If you make the commands multi line with '' then make sure you indent the following lines. example in 'scripts/perftest/runKmeans.sh'
✅
> 9. Remove commented out code, that obviously is not used and should not be used. example in 'scripts/perftest/runStratStats.sh'
The calls for the old algorithms? I thought they might be helpful to compare the output with the new built-ins, but I'll remove them ✅
> 10. Add empty newline in the end of files example 'scripts/perftest/scripts/stratstats.dml'
✅
>
> > ```
> > * I'm assuming the federated tests are to be ran locally only, not on a cluster.
> > ```
>
> True, they are running with localhost workers, We want to add the federated things, but it require a bit of setup and remote machines so not obvious for a perf benchmark.
I've moved/copied some bash logic relating to the federated tests into the fed folder, as I've written new performance tests for als-predict and als-DS. These new perftests were originally not part of the federated tests, which is why I added the "original" versions to the fed folder, to be executed by the fed tests alone.
> > ```
> > * The stratified statistics benchmark needs a heap size of at least 10 GB. This setting is exported only when it's ran locally via "export SYSTEMDS_STANDALONE_OPTS="-Xmx10g -Xms10g -Xmn2000m"
> > ```
> Change this, so that it is set via a settings file, or other means.
I've made a file "env-variables", which is sourced once in runAll.sh. The problem here is, if the federated test is ran with that config (from runAll.sh), then each worker then has that heap size (e.g. 10GB), which crashes the test in my case. I've added that "export" commented to the file.
> > ```
> > * It does not stop when ran with logistic regression data.
> > ```
>
> hmm, this might be a bug, please if you want to, report it as such in JIRA with a way to reproduce it.
>
> https://issues.apache.org/jira/secure/Dashboard.jspa?selectPageId=12335852
Will do.
Also, Merry Christmas! 🎅
--
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 #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on pull request #1481:
URL: https://github.com/apache/systemds/pull/1481#issuecomment-992732966
> Couple of notes, let me know what you think:
> * I've added an extra dot to data generation scripts, to go back a directory, if the command is "systemds". This is done to compensate the different folder structure of the perftests when ran locally, instead of on a cluster.
Don't like this since it implies a different folder to run the scripts from, as seen in the comments above.
> * I'm assuming the federated tests are to be ran locally only, not on a cluster.
True, they are running with localhost workers, We want to add the federated things, but it require a bit of setup and remote machines so not obvious for a perf benchmark.
> * The stratified statistics benchmark needs a heap size of at least 10 GB. This setting is exported only when it's ran locally via "export SYSTEMDS_STANDALONE_OPTS="-Xmx10g -Xms10g -Xmn2000m"
Change this, so that it is set via a settings file, or other means.
> * The K-means built-in function does a predict implicitly, meaning the benchmark predicts twice, as of now.
For now i think we just ignore this.
> * Could not get the decision tree training built-in function to run.
Okay, then leave it out commented and focus on the other parts.
> * It does not stop when ran with logistic regression data.
hmm, this might be a bug, please if you want to, report it as such in JIRA with a way to reproduce it.
https://issues.apache.org/jira/secure/Dashboard.jspa?selectPageId=12335852
> * There are scripts to generate decision tree data, but these can not be ran either as the built-in transform function does not exist anymore, apparently.
> * Could also not get the old decision tree and random forest algorithms running without looking deeper into it. (null pointer exception)
Here leave them out commented, the decision tree and random forest have to be worked on anyway..
--
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] mboehm7 commented on pull request #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
Posted by GitBox <gi...@apache.org>.
mboehm7 commented on pull request #1481:
URL: https://github.com/apache/systemds/pull/1481#issuecomment-1003422103
great to see the progress here - out of curiosity I ran the PCA scenario on one of our small scale-out nodes, and here are the stats (it's not an issue of TSMM but only Eigen):
```
Total elapsed time: 112.377 sec.
Total compilation time: 1.758 sec.
Total execution time: 110.619 sec.
Number of compiled Spark inst: 7.
Number of executed Spark inst: 0.
Cache hits (Mem/Li/WB/FS/HDFS): 43/0/0/0/0.
Cache writes (Li/WB/FS/HDFS): 3/14/0/2.
Cache times (ACQr/m, RLS, EXP): 0.001/0.001/0.006/4.679 sec.
HOP DAGs recompiled (PRED, SB): 0/3.
HOP DAGs recompile time: 0.005 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.
Total JIT compile time: 4.689 sec.
Total JVM GC count: 4.
Total JVM GC time: 0.15 sec.
Heavy hitter instructions:
# Instruction Time(s) Count
1 m_pca 105.247 1
2 eigen 103.240 1
3 write 4.679 2
4 ba+* 0.989 2
5 tsmm 0.455 1
6 / 0.302 5
7 rand 0.287 8
8 + 0.233 10
9 uacmean 0.136 1
10 leftIndex 0.133 3
```
--
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 #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on pull request #1481:
URL: https://github.com/apache/systemds/pull/1481#issuecomment-1002230008
>
If you want to it would be highly appreciated!
--
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] DavidSandruTU commented on pull request #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
Posted by GitBox <gi...@apache.org>.
DavidSandruTU commented on pull request #1481:
URL: https://github.com/apache/systemds/pull/1481#issuecomment-1002206303
Hello @Baunsgaard !
> Thanks for the contribution, 🥇 , i will take it over from here addressing the remaining points and merge it shortly.
I did not write the "executed from correct folder" checks yet, nor did I check out the matrix mult/transpose script if they were still up to date, as I thought I should wait for your response first.
May I implement them or will you take these over too?
--
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] DavidSandruTU edited a comment on pull request #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
Posted by GitBox <gi...@apache.org>.
DavidSandruTU edited a comment on pull request #1481:
URL: https://github.com/apache/systemds/pull/1481#issuecomment-1000563066
> Hi, @DavidSandruTU !
Thanks for the feedback, @Baunsgaard !
> This is a good start, i have only minor things, but each take a bit of time. Just reach out if there is questions, also please try to run it once you have the limit argument just with the smallest size of each data, and post some of the numbers in the comments :), especially if you find some of them interesting or weird.
Following tests were performed on an old intel i5-6500 processor
Most algorithms execute in similar time of a couple of seconds, but here are some outliers:
**StratifiedStatistics on temp/stratstats/A_10k/data: 39.069242381**
PCA is especially heavy hitting it seems, but in line with the algorithm I think(?):
**PCA on temp/dimensionreduction/pcaData5k_2k_dense: 285.998737102**
ALS seems to generally be somewhat heavy hitting (~20-30 seconds), but a more interesting outlier is the direct solve algorithm on sparse data. It should perform better in theory, with optimizations, than the same algorithm with dense data.
**ALS-DS algorithm on temp/als/X10k_1k_sparse: 132.269487990**
> 1. Make all scripts executable from the same directory starting point.
> i suggest, 'scripts/perftest' and write an error if you are not in a directory named that.
Alright, here I want to make sure I've understood everything correctly. If I remember right, a single perftest folder, with all necessary logic, is copied onto the cluster and executed. I think this is why Matthias changed the data generation scripts, in a previous commit, to execute the DMLs in "./datagen/...", instead of "../datagen/...".
What should I do here? Should I assume the datagen scripts are in the perftest folder or outside of it? Want me to add a copy of the datagen scripts to the perftest folder?
Also, I'd then make sure the scripts are in the current directory 'perftest', as I think there would be no 'scripts' parent directory. Am I right in that assumption?
> 2. Suggestion, make all the code available tor scaling experiments in all cases, but add an argument that specify max memory. For instance 'genClusteringData.sh 80' indicating 80MB max, and make this consistent across scripts.
✅
Please check my code and let me know if you like my approach. E.g. genBinomialData.sh and runAllBinomial.sh
> 3. Go through your changes and change all the &>> to &> and >> to >, I am a bit confused about the '&' part of it, since i don't know what it does in context to '>'. If you can explain it it would be nice, otherwise remove the '&' from the piping to files. (this comment is unrelated to adding '&' at the end of the lines, since i know this parallelize the instruction)
✅
I think the "&" is so that error output is also put into the file, not only the standard out. I kept it that way from the original scripts, but never confirmed it myself.
Most outputs are changed to now overwrite, not append, except for some special cases:
- The federated algorithm often accesses a file multiple times, e.g. once when creating and once when killing workers. Here I append instead of overwrite.
- Similar with results/times.txt, where all algorithms append their performance times.
> 4. Don't change permissions on log4j files and DML files.
> You have done so on:
>
> * scripts/perftest/conf/log4j.properties
> * scripts/perftest/fed/data/splitAndMakeFederated.dml
> * scripts/perftest/scripts/GLM-predict.dml
> * scripts/perftest/scripts/alsCG.dml
✅
Today I learned, Git commits permissions.
> 5. Make some of the smallest data generation run in parallel, this is simple to add with a & on the end of the line in sh, and then add a 'wait' in the end of the scripts.
✅
Some scripts have commands that split data into test and training data. These depend on the previous data generation command. I've added multiple waits with PIDs for that, let me know if you're happy with that. E.g. genBinomialData.sh
> 6. Run all... should run all, don't comment anything out. but add an argument to stop execution based on the max Memory argument
Do you want me to uncomment the matrix multiply and transform benchmark too, or was that just meant in context of the max memory argument?
> 7. Make all the loops go though all datasets, subject to the memory constraints, example 'scripts/perftest/runAllMultinominal.sh'
✅
This should be related to point 2. and 6., and is implemented.
> 8. If you make the commands multi line with '' then make sure you indent the following lines. example in 'scripts/perftest/runKmeans.sh'
✅
> 9. Remove commented out code, that obviously is not used and should not be used. example in 'scripts/perftest/runStratStats.sh'
The calls for the old algorithms? I thought they might be helpful to compare the output with the new built-ins, but I'll remove them ✅
> 10. Add empty newline in the end of files example 'scripts/perftest/scripts/stratstats.dml'
✅
>
> > ```
> > * I'm assuming the federated tests are to be ran locally only, not on a cluster.
> > ```
>
> True, they are running with localhost workers, We want to add the federated things, but it require a bit of setup and remote machines so not obvious for a perf benchmark.
I've moved/copied some bash logic relating to the federated tests into the fed folder, as I've written new performance tests for als-predict and als-DS. These new perftests were originally not part of the federated tests, which is why I added the "original" versions to the fed folder, to be executed by the fed tests alone.
> > ```
> > * The stratified statistics benchmark needs a heap size of at least 10 GB. This setting is exported only when it's ran locally via "export SYSTEMDS_STANDALONE_OPTS="-Xmx10g -Xms10g -Xmn2000m"
> > ```
> Change this, so that it is set via a settings file, or other means.
I've made a file "env-variables", which is sourced once in runAll.sh. The problem here is, if the federated test is ran with that config (from runAll.sh), then each worker then has that heap size (e.g. 10GB), which crashes the test in my case. I've added that "export" commented to the file.
> > ```
> > * It does not stop when ran with logistic regression data.
> > ```
>
> hmm, this might be a bug, please if you want to, report it as such in JIRA with a way to reproduce it.
>
> https://issues.apache.org/jira/secure/Dashboard.jspa?selectPageId=12335852
Will do.
Also, Merry Christmas! 🎅
--
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] mboehm7 commented on pull request #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
Posted by GitBox <gi...@apache.org>.
mboehm7 commented on pull request #1481:
URL: https://github.com/apache/systemds/pull/1481#issuecomment-1003424325
For PCA with many columns we have PPCA, which runs this scenario as follows:
```
Total elapsed time: 6.880 sec.
Total compilation time: 1.586 sec.
Total execution time: 5.293 sec.
Number of compiled Spark inst: 0.
Number of executed Spark inst: 0.
Cache hits (Mem/Li/WB/FS/HDFS): 21/0/0/0/0.
Cache writes (Li/WB/FS/HDFS): 3/6/0/2.
Cache times (ACQr/m, RLS, EXP): 0.000/0.000/0.004/4.511 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.
Total JIT compile time: 3.13 sec.
Total JVM GC count: 4.
Total JVM GC time: 0.138 sec.
Heavy hitter instructions:
# Instruction Time(s) Count
1 write 4.511 2
2 rand 0.324 7
3 + 0.256 12
4 leftIndex 0.161 3
5 / 0.025 1
6 n+ 0.017 1
7 * 0.015 5
8 rmvar 0.007 15
9 ceil 0.004 2
10 createvar 0.001 21
```
Similar to lm and als, we should dispatch to the appropriate pca algorithm based on heuristics. 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] Baunsgaard commented on pull request #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on pull request #1481:
URL: https://github.com/apache/systemds/pull/1481#issuecomment-1003060944
> Changes are up.
>
> I also have a commit ready that reproduces the indefinitely running decision tree, with scripts in the perftest/todo directory. I'd reference them in the JIRA. May I push it?
yes just push it, i might move it a bit around to another PR/commit but it would be nice to have 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
[GitHub] [systemds] DavidSandruTU commented on pull request #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
Posted by GitBox <gi...@apache.org>.
DavidSandruTU commented on pull request #1481:
URL: https://github.com/apache/systemds/pull/1481#issuecomment-1003033522
Changes are up.
I also have a commit ready that reproduces the indefinitely running decision tree with scripts in the perftest/todo directory. I'd reference them in the JIRA.
May I push it?
--
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 change in pull request #1481: [SYSTEMDS-2832] Refactoring of old performance benchmarks.
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on a change in pull request #1481:
URL: https://github.com/apache/systemds/pull/1481#discussion_r767977164
##########
File path: scripts/perftest/fed/runALSFed.sh
##########
@@ -35,24 +35,28 @@ trap 'err_report $LINENO' ERR
export SYSDS_QUIET=1
BASEPATH=$(dirname "$0")
+TEMPFILENAME=$(basename -- "$FILENAME")
+BASEFILENAME=${TEMPFILENAME%.*}
-${BASEPATH}/../genALSData.sh systemds $DATADIR; # generate the data
+${BASEPATH}/../genALS_FedData.sh systemds $DATADIR &>> ${BASEPATH}/../logs/genALS_FedData.out; # generate the data
# start the federated workers on localhost
-${BASEPATH}/utils/startFedWorkers.sh systemds $DATADIR $NUMFED "localhost";
+"date" &>> ${BASEPATH}/../logs/runAllFed.out
+${BASEPATH}/utils/startFedWorkers.sh systemds $DATADIR $NUMFED "localhost" &>> ${BASEPATH}/../logs/runAllFed.out;
Review comment:
don't append but overwrite
##########
File path: scripts/perftest/fed/runALSFed.sh
##########
@@ -35,24 +35,28 @@ trap 'err_report $LINENO' ERR
export SYSDS_QUIET=1
BASEPATH=$(dirname "$0")
+TEMPFILENAME=$(basename -- "$FILENAME")
+BASEFILENAME=${TEMPFILENAME%.*}
-${BASEPATH}/../genALSData.sh systemds $DATADIR; # generate the data
+${BASEPATH}/../genALS_FedData.sh systemds $DATADIR &>> ${BASEPATH}/../logs/genALS_FedData.out; # generate the data
# start the federated workers on localhost
-${BASEPATH}/utils/startFedWorkers.sh systemds $DATADIR $NUMFED "localhost";
+"date" &>> ${BASEPATH}/../logs/runAllFed.out
Review comment:
"date"
is equivalent to
date
##########
File path: scripts/perftest/genALS_FedData.sh
##########
@@ -0,0 +1,50 @@
+#!/bin/bash
+#-------------------------------------------------------------
+#
+# 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.
+#
+#-------------------------------------------------------------
+
Review comment:
make same comment on location here.
##########
File path: scripts/perftest/fed/runALSFed.sh
##########
@@ -35,24 +35,28 @@ trap 'err_report $LINENO' ERR
export SYSDS_QUIET=1
BASEPATH=$(dirname "$0")
+TEMPFILENAME=$(basename -- "$FILENAME")
+BASEFILENAME=${TEMPFILENAME%.*}
-${BASEPATH}/../genALSData.sh systemds $DATADIR; # generate the data
+${BASEPATH}/../genALS_FedData.sh systemds $DATADIR &>> ${BASEPATH}/../logs/genALS_FedData.out; # generate the data
# start the federated workers on localhost
-${BASEPATH}/utils/startFedWorkers.sh systemds $DATADIR $NUMFED "localhost";
+"date" &>> ${BASEPATH}/../logs/runAllFed.out
Review comment:
don't append but overwrite
##########
File path: scripts/perftest/genBinomialData.sh
##########
@@ -19,40 +19,42 @@
# under the License.
#
#-------------------------------------------------------------
-
CMD=$1
Review comment:
same comment on location here.
##########
File path: scripts/perftest/genALSData.sh
##########
@@ -21,30 +21,31 @@
#-------------------------------------------------------------
CMD=$1
-DATADIR=$2
+DATADIR=$2/als
-FORMAT="binary" # can be csv, mm, text, binary
+FORMAT="text" # can be csv, mm, text, binary
DENSE_SP=0.9
SPARSE_SP=0.01
-BASEPATH=$(dirname $0)
+EXTRADOT=""
+if [ "${CMD}" == "systemds" ]; then EXTRADOT="." ; fi
#generate XS scenarios (80MB)
-${CMD} -f ${BASEPATH}/../datagen/genRandData4ALS.dml --nvargs X=${DATADIR}/X10k_1k_dense rows=10000 cols=1000 rank=10 nnz=`echo "scale=0; 10000 * 1000 * $DENSE_SP" | bc` sigma=0.01 fmt=$FORMAT
-${CMD} -f ${BASEPATH}/../datagen/genRandData4ALS.dml --nvargs X=${DATADIR}/X10k_1k_sparse rows=10000 cols=1000 rank=10 nnz=`echo "scale=0; 10000 * 1000 * $SPARSE_SP" | bc` sigma=0.01 fmt=$FORMAT
+${CMD} -f ${EXTRADOT}./datagen/genRandData4ALS.dml --nvargs X=${DATADIR}/X10k_1k_dense rows=10000 cols=1000 rank=10 nnz=`echo "scale=0; 10000 * 1000 * $DENSE_SP" | bc` sigma=0.01 fmt=$FORMAT
Review comment:
I'm pretty sure that this change is not needed, since the script is intended to be run one directory further in,
I would suggest that you add a comment int the top specifying where to run it from, and even better an error that says, "wrong directory, please cd to xyz"
--
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