Skip to content

Conversation

@jangorecki
Copy link
Owner

@jangorecki jangorecki commented May 13, 2025

NA handling in frollmax added
frollmax code reorg
more tests coming

@jangorecki jangorecki requested review from aitap and ben-schwen May 13, 2025 15:22
adaptive = TRUE
}
if (isTRUE(adaptive) && base::getRversion() < "3.4.0") ## support SET_GROWABLE_BIT
stopf("frollapply adaptive=TRUE requires at least R 3.4.0"); # nocov
Copy link
Collaborator

@MichaelChirico MichaelChirico May 13, 2025

Choose a reason for hiding this comment

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

let's just bump to 3.4.0 dependency for 1.18.0: Rdatatable#6840

Copy link
Owner Author

Choose a reason for hiding this comment

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

this will be rewritten in Rdatatable#5575 and then it will also work on < 3.4.0 but with an extra copy on each iteration.

rev2 = function(x) if (is.list(x)) lapply(x, rev) else rev(x)
if (verbose)
cat("froll: adaptive=TRUE && align='left' pre-processing for align='right'\n")
cat("frollapply: adaptive=TRUE && align='left' pre-processing for align='right'\n")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
cat("frollapply: adaptive=TRUE && align='left' pre-processing for align='right'\n")
catf("frollapply: adaptive=TRUE && align='left' pre-processing for align='right'\n")

Copy link
Owner Author

Choose a reason for hiding this comment

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

There are multiple places for updates like that. As I move forward with next PRs on each one there is more and more conflicts to resolve, conflicts accumulates. Therefore to avoid more conflicts for next PRs I will put it on a list of follow up things to do, once all PRs are in.

@jangorecki jangorecki merged commit b2870e6 into froll2025 May 14, 2025
8 of 9 checks passed
}
}
}
static inline void windowmaxnarm(const double * restrict x, uint64_t o, int k, bool narm, int *nc, double * restrict w, uint64_t *iw) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

static that was lost here, due to auto resolving conflicts, will be added in froll2025max8, where it was originally included

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.

3 participants