Skip to content

Optimize similarityMatrix and templateIdxMatrix functions - #8

Open
chinandrew wants to merge 5 commits into
martakarass:masterfrom
chinandrew:optimize_similarity
Open

chinandrew wants to merge 5 commits into
martakarass:masterfrom
chinandrew:optimize_similarity

Conversation

@chinandrew

@chinandrew chinandrew commented May 22, 2023

Copy link
Copy Markdown

A number of changes here broken down by commit:

05d0436 just adds the faster C++ versions

ab05662 combines the similiartyMatrix and templateIdxMatrix functions, which were repeating the sliding_cov/cor functions twice. Instead, a custom implementation of pmax that returns both the max and the index of the max at once is used. After this change, essentially getting the template idx is free. The custom implementation isn't actually needed; we could just combine the pmax and max.col into the same lapply at the expense of a second or two of performance in this example.

747b526 removes the compute.template.idx argument and associated documentation, though it hasn't been rebuilt in this commit. It's possible I missed a few examples.

67c6c3e adds a check for ~0 standard deviations in the correlation computation, which previously would not be caught and could lead to spurious correlations due to floating point error when doing 0/0. Now it returns NA if the sd <1e-10 (i picked this threshold arbitrarily). Performance change is negligible.

8cbdf0b makes a change to the sliding correlation computation which stores the computed standard deviations from the first template for use in later templates. It's also relatively minor, and depends on how many templates you run. If you only run 1 template there's no speedup, and if you run more the speedup increase (still not by much though, maybe ~10%?). I could do without this commit since it does add more complexity to the code.

I think my main notes are:

  1. Deciding if its worth using the custom Rcpp function for ab05662 or using the old two R functions at the slight cost of performance. Basically it's probably easier to maintain less Rcpp (though I think the Rcpp function is a bit cleaner).
  2. Deciding if 8cbdf0b is even worth including. Another one of these complexity/performance tradeoffs.
  3. Documentation needs to be reviewed and rebuilt from 747b526. I tried to capture all the references in code but have no re-read/built all the compiled vignettes or html docs to.
  4. Sometimes, the Rcpp functions can be slow if your C++ compiler is not set to compile them in the right way. This hasn't been an issue with anyones laptop as far as I know, but JHPCE required updating a Makevars file to get it to compile in the fast way (I didn't check if all other Rcpp functions in other packages were also slow before I made this change). I'm not too familiar with how CRAN handles building the package, but just noting that there's this extra dependency that Rcpp introduces. Perhaps someone with more expertise here can advise on if there's anything extra we need to do for CRAN, but from a brief search I couldn't find anything.

Profiling:

Before:
before

After 05d0436 (note similarityMatrix and templateIdxMatrix times)
after_1

After ab05662
after_2

After 8cbdf0b
after_3

@chinandrew
chinandrew force-pushed the optimize_similarity branch from b6b7ca6 to 4e68cc1 Compare May 22, 2023 16:03
@chinandrew
chinandrew force-pushed the optimize_similarity branch from 4e68cc1 to 05d0436 Compare May 22, 2023 16:22
@chinandrew
chinandrew force-pushed the optimize_similarity branch from 37f7959 to 67c6c3e Compare May 22, 2023 19:18
Comment thread src/pmaxIdx.cpp
//
// Adapted from https://stackoverflow.com/a/66020829
//
// Vectors should be of the same length. Otherwise, will only go up to the

@chinandrew chinandrew May 22, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seemed like this requirement was not an issue. If not, we should revert to the base R functions instead of this custom function. Another reason to do so much just be because it's easier to maintain fewer Rcpp functions and we're ok with slowing down by ~3% (just my guesstimate).

@chinandrew chinandrew changed the title [DRAFT] Optimize similarityMatrix and templateIdxMatrix functions Optimize similarityMatrix and templateIdxMatrix functions May 22, 2023
@chinandrew
chinandrew marked this pull request as ready for review May 22, 2023 19:38
@muschellij2

Copy link
Copy Markdown
Collaborator
library(dplyr)
#> 
#> Attaching package: 'dplyr'
#> The following objects are masked from 'package:stats':
#> 
#>     filter, lag
#> The following objects are masked from 'package:base':
#> 
#>     intersect, setdiff, setequal, union
library(microbenchmark)
template = adeptdata::stride_template$left_wrist[[1]][1,]

xyz = adeptdata::acc_walking_IU
xyz = xyz %>% 
  filter(loc_id == "left_wrist") %>% 
  filter(subj_id == subj_id[1])
xyz = xyz %>% 
  select(x, y, z)

remove.packages("adept")
#> Removing package from '/Library/Frameworks/R.framework/Versions/4.2/Resources/library'
#> (as 'lib' is unspecified)
remotes::install_github("martakarass/adept")
#> Using github PAT from envvar GITHUB_PAT
#> Downloading GitHub repo martakarass/adept@HEAD
#> 
#> ── R CMD build ─────────────────────────────────────────────────────────────────
#> * checking for file ‘/private/var/folders/1s/wrtqcpxn685_zk570bnx9_rr0000gr/T/Rtmp6WWcJH/remotesa84f2688fcf7/martakarass-adept-d7165ad/DESCRIPTION’ ... OK
#> * preparing ‘adept’:
#> * checking DESCRIPTION meta-information ... OK
#> * checking for LF line-endings in source and make files and shell scripts
#> * checking for empty or unneeded directories
#> Omitted ‘LazyData’ from DESCRIPTION
#> * building ‘adept_1.2.tar.gz’
#> Adding 'adept_1.2.tgz' to the cache

microbenchmark(
  adept::segmentWalking(
    xyz = xyz,
    xyz.fs = 100,
    template = template)
)
#> Unit: milliseconds
#>                                                                 expr      min
#>  adept::segmentWalking(xyz = xyz, xyz.fs = 100, template = template) 701.4646
#>        lq     mean  median       uq      max neval
#>  731.4657 769.7468 748.205 767.9314 1993.847   100
unloadNamespace("adept")

remove.packages("adept")
#> Removing package from '/Library/Frameworks/R.framework/Versions/4.2/Resources/library'
#> (as 'lib' is unspecified)
remotes::install_github("chinandrew/adept@optimize_similarity")
#> Using github PAT from envvar GITHUB_PAT
#> Downloading GitHub repo chinandrew/adept@optimize_similarity
#> 
#> ── R CMD build ─────────────────────────────────────────────────────────────────
#> * checking for file ‘/private/var/folders/1s/wrtqcpxn685_zk570bnx9_rr0000gr/T/Rtmp6WWcJH/remotesa84f6553a00c/chinandrew-adept-8cbdf0b/DESCRIPTION’ ... OK
#> * preparing ‘adept’:
#> * checking DESCRIPTION meta-information ... OK
#> * cleaning src
#> * checking for LF line-endings in source and make files and shell scripts
#> * checking for empty or unneeded directories
#> Omitted ‘LazyData’ from DESCRIPTION
#> * building ‘adept_1.2.tar.gz’

microbenchmark(
  adept::segmentWalking(
    xyz = xyz,
    xyz.fs = 100,
    template = template)
)
#> Unit: seconds
#>                                                                 expr      min
#>  adept::segmentWalking(xyz = xyz, xyz.fs = 100, template = template) 4.778391
#>        lq     mean   median       uq      max neval
#>  4.843496 4.907314 4.880141 4.922944 5.492787   100
unloadNamespace("adept")

Created on 2023-08-09 with reprex v2.0.2

Session info
sessioninfo::session_info()
#> ─ Session info ───────────────────────────────────────────────────────────────
#>  setting  value
#>  version  R version 4.2.1 (2022-06-23)
#>  os       macOS Big Sur ... 10.16
#>  system   x86_64, darwin17.0
#>  ui       X11
#>  language (EN)
#>  collate  en_US.UTF-8
#>  ctype    en_US.UTF-8
#>  tz       America/New_York
#>  date     2023-08-09
#>  pandoc   3.1.5 @ /usr/local/bin/ (via rmarkdown)
#> 
#> ─ Packages ───────────────────────────────────────────────────────────────────
#>  package        * version    date (UTC) lib source
#>  adeptdata        1.1        2021-03-28 [1] CRAN (R 4.2.0)
#>  bit              4.0.5      2022-11-15 [1] CRAN (R 4.2.0)
#>  bit64            4.0.5      2020-08-30 [1] CRAN (R 4.2.0)
#>  blob             1.2.4      2023-03-17 [1] CRAN (R 4.2.0)
#>  cachem           1.0.8      2023-05-01 [1] CRAN (R 4.2.0)
#>  callr            3.7.3      2022-11-02 [1] CRAN (R 4.2.0)
#>  cli              3.6.1      2023-03-23 [1] CRAN (R 4.2.0)
#>  codetools        0.2-19     2023-02-01 [1] CRAN (R 4.2.0)
#>  colorspace       2.1-0      2023-01-23 [1] CRAN (R 4.2.0)
#>  crancache        0.0.0.9001 2022-05-13 [1] Github (r-lib/crancache@7ea4e47)
#>  cranlike         1.0.2      2018-11-26 [1] CRAN (R 4.2.0)
#>  crayon           1.5.2      2022-09-29 [1] CRAN (R 4.2.0)
#>  curl             5.0.1      2023-06-07 [1] CRAN (R 4.2.0)
#>  data.table       1.14.8     2023-02-17 [1] CRAN (R 4.2.0)
#>  DBI              1.1.3      2022-06-18 [1] CRAN (R 4.2.0)
#>  debugme          1.1.0      2017-10-22 [1] CRAN (R 4.2.0)
#>  desc             1.4.2      2022-09-08 [1] CRAN (R 4.2.0)
#>  digest           0.6.33     2023-07-07 [1] CRAN (R 4.2.0)
#>  dplyr          * 1.1.2      2023-04-20 [1] CRAN (R 4.2.0)
#>  dvmisc           1.1.4      2019-12-16 [1] CRAN (R 4.2.0)
#>  evaluate         0.21       2023-05-05 [1] CRAN (R 4.2.0)
#>  fansi            1.0.4      2023-01-22 [1] CRAN (R 4.2.0)
#>  fastmap          1.1.1      2023-02-24 [1] CRAN (R 4.2.0)
#>  fs               1.6.3      2023-07-20 [1] CRAN (R 4.2.0)
#>  generics         0.1.3      2022-07-05 [1] CRAN (R 4.2.0)
#>  ggplot2          3.4.2      2023-04-03 [1] CRAN (R 4.2.0)
#>  glue             1.6.2      2022-02-24 [1] CRAN (R 4.2.0)
#>  gtable           0.3.3      2023-03-21 [1] CRAN (R 4.2.0)
#>  htmltools        0.5.5      2023-03-23 [1] CRAN (R 4.2.0)
#>  httr             1.4.6      2023-05-08 [1] CRAN (R 4.2.0)
#>  kableExtra       1.3.4      2021-02-20 [1] CRAN (R 4.2.0)
#>  knitr            1.43       2023-05-25 [1] CRAN (R 4.2.0)
#>  lattice          0.21-8     2023-04-05 [1] CRAN (R 4.2.0)
#>  lifecycle        1.0.3      2022-10-07 [1] CRAN (R 4.2.0)
#>  magrittr         2.0.3      2022-03-30 [1] CRAN (R 4.2.0)
#>  MASS             7.3-60     2023-05-04 [1] CRAN (R 4.2.0)
#>  Matrix           1.6-0      2023-07-08 [1] CRAN (R 4.2.1)
#>  memoise          2.0.1      2021-11-26 [1] CRAN (R 4.2.0)
#>  microbenchmark * 1.4.10     2023-04-28 [1] CRAN (R 4.2.0)
#>  mitools          2.4        2019-04-26 [1] CRAN (R 4.2.0)
#>  multcomp         1.4-25     2023-06-20 [1] CRAN (R 4.2.0)
#>  munsell          0.5.0      2018-06-12 [1] CRAN (R 4.2.0)
#>  mvtnorm          1.2-2      2023-06-08 [1] CRAN (R 4.2.0)
#>  parsedate        1.3.1      2022-10-27 [1] CRAN (R 4.2.0)
#>  pillar           1.9.0      2023-03-22 [1] CRAN (R 4.2.0)
#>  pkgbuild         1.4.2      2023-06-26 [1] CRAN (R 4.2.0)
#>  pkgconfig        2.0.3      2019-09-22 [1] CRAN (R 4.2.0)
#>  pracma           2.4.2      2022-09-22 [1] CRAN (R 4.2.0)
#>  prettyunits      1.1.1      2020-01-24 [1] CRAN (R 4.2.0)
#>  processx         3.8.2      2023-06-30 [1] CRAN (R 4.2.0)
#>  ps               1.7.5      2023-04-18 [1] CRAN (R 4.2.0)
#>  purrr            1.0.1      2023-01-10 [1] CRAN (R 4.2.0)
#>  R.cache          0.16.0     2022-07-21 [1] CRAN (R 4.2.0)
#>  R.methodsS3      1.8.2      2022-06-13 [1] CRAN (R 4.2.0)
#>  R.oo             1.25.0     2022-06-12 [1] CRAN (R 4.2.0)
#>  R.utils          2.12.2     2022-11-11 [1] CRAN (R 4.2.0)
#>  R6               2.5.1      2021-08-19 [1] CRAN (R 4.2.0)
#>  rappdirs         0.3.3      2021-01-31 [1] CRAN (R 4.2.0)
#>  rbenchmark       1.0.0      2012-08-30 [1] CRAN (R 4.2.0)
#>  Rcpp             1.0.11     2023-07-06 [1] CRAN (R 4.2.0)
#>  rematch2         2.1.2      2020-05-01 [1] CRAN (R 4.2.0)
#>  remotes          2.4.2.1    2023-07-18 [1] CRAN (R 4.2.0)
#>  reprex           2.0.2      2022-08-17 [1] CRAN (R 4.2.0)
#>  rlang            1.1.1      2023-04-28 [1] CRAN (R 4.2.0)
#>  rmarkdown        2.23       2023-07-01 [1] CRAN (R 4.2.0)
#>  rprojroot        2.0.3      2022-04-02 [1] CRAN (R 4.2.0)
#>  RSQLite          2.3.1      2023-04-03 [1] CRAN (R 4.2.0)
#>  rstudioapi       0.15.0     2023-07-07 [1] CRAN (R 4.2.0)
#>  rvest            1.0.3      2022-08-19 [1] CRAN (R 4.2.0)
#>  sandwich         3.0-2      2022-06-15 [1] CRAN (R 4.2.0)
#>  scales           1.2.1      2022-08-20 [1] CRAN (R 4.2.0)
#>  sessioninfo      1.2.2      2021-12-06 [1] CRAN (R 4.2.0)
#>  stringi          1.7.12     2023-01-11 [1] CRAN (R 4.2.0)
#>  stringr          1.5.0      2022-12-02 [1] CRAN (R 4.2.0)
#>  styler           1.10.1     2023-06-05 [1] CRAN (R 4.2.0)
#>  survey           4.2-1      2023-05-03 [1] CRAN (R 4.2.0)
#>  survival         3.5-5      2023-03-12 [1] CRAN (R 4.2.0)
#>  svglite          2.1.1      2023-01-10 [1] CRAN (R 4.2.0)
#>  systemfonts      1.0.4      2022-02-11 [1] CRAN (R 4.2.0)
#>  tab              5.1.1      2021-08-02 [1] CRAN (R 4.2.0)
#>  TH.data          1.1-2      2023-04-17 [1] CRAN (R 4.2.0)
#>  tibble           3.2.1      2023-03-20 [1] CRAN (R 4.2.0)
#>  tidyselect       1.2.0      2022-10-10 [1] CRAN (R 4.2.0)
#>  utf8             1.2.3      2023-01-31 [1] CRAN (R 4.2.0)
#>  vctrs            0.6.3      2023-06-14 [1] CRAN (R 4.2.0)
#>  viridisLite      0.4.2      2023-05-02 [1] CRAN (R 4.2.0)
#>  webshot          0.5.5      2023-06-26 [1] CRAN (R 4.2.0)
#>  withr            2.5.0      2022-03-03 [1] CRAN (R 4.2.0)
#>  xfun             0.39       2023-04-20 [1] CRAN (R 4.2.0)
#>  xml2             1.3.5      2023-07-06 [1] CRAN (R 4.2.0)
#>  yaml             2.3.7      2023-01-23 [1] CRAN (R 4.2.0)
#>  zoo              1.8-12     2023-04-13 [1] CRAN (R 4.2.0)
#> 
#>  [1] /Library/Frameworks/R.framework/Versions/4.2/Resources/library
#> 
#> ──────────────────────────────────────────────────────────────────────────────

@chinandrew

Copy link
Copy Markdown
Author

This discrepancy might be a makevars issue, i've noticed if it's not set correctly when compiling the Rcpp it can be slower.

@muschellij2

Copy link
Copy Markdown
Collaborator

I'm not 100% sure htat the similarityMatrix inputs are the same from this and the ADEPT master branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants