-
Notifications
You must be signed in to change notification settings - Fork 1k
new rolling functions: min and prod #7299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Generated via commit 227b39e 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 #7299 +/- ##
==========================================
+ Coverage 99.08% 99.09% +0.01%
==========================================
Files 83 83
Lines 15781 16052 +271
==========================================
+ Hits 15636 15907 +271
Misses 145 145 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
} | ||
} | ||
|
||
static inline void wmin(const double * restrict x, uint64_t o, int k, double * restrict w, uint64_t *iw, bool narm) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a big fan of macros but couldn't we do something like and deduplicate wmin
and wmax
?
#define DEFINE_WINDOWFUN(fname, init_val, op)
DEFINE_WINDOWFUN(wmax, R_NegInf, >=)
DEFINE_WINDOWFUN(wmin, R_PosInf, <=)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be, one reason not to is reduced code coverage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same holds for frollmaxFast
and frollmaxExact
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same holds for
frollmaxFast
andfrollmaxExact
frollmax Fast vs. Exact does not have that much in common, you possibly meant froll Min vs Max
as for the main functions, to turn them into macros would require __func__
handling as well. Additionally it would reduce clarify of code, and friendliness for code editors.
As for the helper functions wmin, wmax, IMO it indeed make sense, but maybe better as a follow up. So this PR will be focused more on adding new things rather than refactoring previous ones.
Two new rolling functions, towards #2778.
Supersede #5682
Optimizations
benchmark vs very fast
roll
packageThis is for
rnorm
, be aware that for specific input, data.table's implementation can be much slower, see issue reported for rolling max #5923This work was supported by the Medical Research Council, UK [grant number 1573].