Speed up quantile_ensemble_flex by forming model$A with one sparseMatrix call and specially treating tau_group runs
#8
Conversation
Keep track of the i,j,x parts using lists of vectors rather than Matrix objects, then call sparseMatrix only once, in order to reduce the amount of checking performed, and to prevent unintended conversions between Tsparse and Csparse forms.
When `tau_groups` has runs (duplicate values that appear consecutively), use only `p` variables to represent the coefficients within each run. Remove noncrossing constraints within the runs when one criterion (nonneg is TRUE and q0 doesn't contain crossings) indicates that removing these contraints should not affect the output. This approach may make quantile_ensemble_flex's `model` look the same or similar to `quantile_ensemble_stand`'s, and `quantile_ensemble_stand` might be sped up by delegating to / post-processing `quantile_ensemble_flex` to take advantage of the A matrix formation changes.
|
@lcbrooks This looks wonderful. Great job and thank you! One thing: please add yourself as a package author! Another thing: I forget what's the easiest way for me to see the results of the tests. I can see the code you've added but can you remind what's the easiest way for me to look at the test outputs (the checks that the old code and the new code match up)? Thanks. |
Update `sparseMatrix` call to use `repr = "T"` rather than deprecated equivalent `giveCsparse=FALSE`. Also format adjacent line.
|
Thanks! I updated the DESCRIPTION above (is "aut" the right "role"?). For running tests, use |
|
Yep "aut" is the right role. I just ran I'm not sure if the only error is due to the last commit (use of |
Context: In the `Matrix` package v1.3-0, `sparseMatrix` was [updated](https://cran.r-project.org/web/packages/Matrix/news.html) with a new argument `repr` and deprecation of `giveCsparse=FALSE`. Commit 9f8fa00 avoided deprecation warnings when using `Matrix` package versions >= 1.3-0, but generated errors with versions < 1.3-0. Action: Avoid both errors for earlier versions and deprecation warnings for later versions by constructing Tsparse matrices by directly constructing an appropriate S4 object.
brookslogan
force-pushed
the
ensemble-speed-up-A-matrix
branch
from
0174dac to
42e269c
Compare
|
Thanks for catching this! It should be fixed now. See the commit message here / above for context. (I did mean |
|
Sorry for the delay here. I'm happy to merge this now (reran tests and all 87 passed!). @elray1 You might consider running this for your use case now and see how fast it is? (No pressure either way, but just curious to see if it's competitive with your Python code now.) |
This PR builds on #6. It provides two extra efficiency gains for
quantile_ensemble_flex:model$Aformation even further by building up parts ofi,j, andxvectors to feed intosparseMatrixbeforehand, then callingsparseMatrixonly once. This avoids extra checks and conversions between Csparse and Tsparse matrices. An Rsparse-inspired approach might be marginally faster, but harder to read.tau_groupscontains runs (consecutive duplicated values): eliminates "duplicate" optimization variables, and when nonneg is TRUE and q0 doesn't contain crossings, eliminates noncrossing constraints within these runs that appear unnecessary. It may be possible to have a competitive or faster alternative to the currentquantile_ensemble_standby delegating to / postprocessing this version ofquantile_ensemble_flex.Additionally, this adds some characterization tests for
quantile_ensembleusingtestthat.Exploratory testing & benchmarking code:
benchmark-flex-matrix-format.md
Selected benchmark results for a 400 x 20 x 7 q matrix with three tau groups, no crossings within
qarr, and default parameters: