add GForce support for outer transformations #7600
Conversation
ben-schwen
added
the
GForce
| # Outer transformations that can wrap GForce-optimizable expressions | ||
| # e.g., sqrt(min(x)) should be optimized to sqrt(gmin(x)) | ||
| # for the moment we only include unary functions | ||
| .gforce_outer_trans = c("sqrt", "abs", "sign", "floor", "ceiling", |
There was a problem hiding this comment.
what goes wrong if we just let any generic foo() through here? As long as the "innermost" function is GForce-able
There was a problem hiding this comment.
Things like min(min(x)) should already be quite problematic...
There was a problem hiding this comment.
Our own test suite dislikes e.g. .Internal(mean(x))
| test(2363.03, optimize=1:2, dt[, sqrt(abs(min(y))), by=x, verbose=TRUE], output=out) | ||
| test(2363.04, optimize=1:2, dt[, sqrt(min(y)) + 1, by=x, verbose=TRUE], output=out) | ||
| test(2363.05, optimize=1:2, dt[, floor(mean(y)), by=x, verbose=TRUE], output=out) | ||
| # Transformation around non-GForce expression should NOT optimize |
There was a problem hiding this comment.
add one for nesting non-GForce calls too
|
No obvious timing issues in HEAD=gforce_outer_transformations Generated via commit b40b684 Download link for the artifact containing the test results: ↓ atime-results.zip
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7600 +/- ##
=======================================
Coverage 99.00% 99.00%
=======================================
Files 87 87
Lines 16893 16893
=======================================
Hits 16725 16725
Misses 168 168 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| "cos", "sin", "tan", "acos", "asin", "atan", | ||
| "cosh", "sinh", "tanh", "acosh", "asinh", "atanh", | ||
| "is.na", "is.nan", "is.finite", "is.infinite") | ||
|
|
There was a problem hiding this comment.
Hello:
By setdiff(methods::getGroupMembers("Math"), .gforce_outer_trans) you can add:
"trunc" "cummax" "cummin" "cumprod" "cumsum"
"cospi" "sinpi" "tanpi"
"gamma" "lgamma" "digamma" "trigamma"
Note "log" can take "base = ..." as first argument. trunc can take ... also.
There was a problem hiding this comment.
The cumulative functions won't work. Maybe unary functions was too unprecise. I have added an "elementwise".
Binary functions will optimize as long ppl provide constantish parameters and not variables for the parameters, e.g. log(min(x), 2) works while t<-2; log(min(x), 2) does not
3 participants
Towards #7594
Implements case 1) of outer transformations as e.g.
sqrt(min(a))andmean(x)^2The GForce rewrite made this a charm 😃