Repository navigation
Release checks #3211
Description
Activity
I notice that significant changes are made recently so I keep up with those on a daily basis. So far the system runs perfectly with 4c9a1f0.
As for the total time, it is unfortunate that fork parallelism is heavily used so that multi-threading falls to single thread and does not recover when fork is ended. Therefore I haven't observed significant difference in computing time so far.
I'll test against the latest commit very soon.
Reacted by Matt DowleThanks @renkun-ken!
We could maybe have a look at fork again. See past problems here :
https://github.com/Rdatatable/data.table/blob/master/src/openmp-utils.c#L73
Are you on Linux/Mac/WIndows? ( I didn't think Windows had fork, but somebody told me recently that it does.)
As those comments say, we used to have an after_fork callback that restored omp to using more than 1 thread. But the problems were on Mac and/or Intel's iomp. It may be possible to try it again just on Linux, if you're on Linux.
Does setDTthreads(0) restore multi-threaded after a parallel fork for you? If so, there's a good chance theafter_forkcould turn it on automatically if we put it back.Latest commit 0daefad is broken here. I'll take a closer look and file an issue about it soon.
The problem lies on an assumed behavior that
dt[TRUE]always creates a shallow copy ofdtso that adding columns to the shallow copy does not influence the original data.table. See #3214.Thanks @renkun-ken. #3214 now fixed by PR #3213 and merged.
@renkun-ken Many thanks for this invaluable testing!!
#3216 fixed, and #2885 (restore multi threading after fork) done too.
Please rerun whenever convenient.Reacted by Kun RenA full run with 76d4a62 is in progress...
Reacted by Matt DowleJust found another issue which seems to be breaking. The updated subsetting tends to under-allocating. See #3228.
Empty
.SDcolsnow produces an error (which I think is by design in recent changes?), following code no longer works:dt <- data.table(x = rnorm(10), y = rnorm(10)) cols <- intersect(c("p", "q"), names(dt)) dt[, x := rowSums(.SD), .SDcols = cols]
As #3229 has just been merged, no other issues are found so far.
Thanks again for the quick turnaround!
Looks like that failed in 1.11.8 too but with a better message. Will make the error message same as before but still an error, right?
> dt <- data.table(x = rnorm(10), y = rnorm(10)) > cols <- intersect(c("p", "q"), names(dt)) v1.11.8 ====== > dt[, x := rowSums(.SD), .SDcols = cols] Error in `[.data.table`(dt, , `:=`(x, rowSums(.SD)), .SDcols = cols) : RHS of assignment to existing column 'x' is zero length but not NULL. If you intend to delete the column use NULL. Otherwise, the RHS must have length > 0; e.g., NA_integer_. If you are trying to change the column type to be an empty list column then, as with all column type changes, provide a full length RHS vector such as vector('list',nrow(DT)); i.e., 'plonk' in the new column. dev error not intended ====== > dt[, x := rowSums(.SD), .SDcols = cols] Error in `[.data.table`(dt, , `:=`(x, rowSums(.SD)), .SDcols = cols) : Empty .SDcols is not yet supported.
Sorry, I just noticed that there might be a bug in 1.11.8 on empty
.SDcolsthat I happended to take advantage of unintentionally.The following code works with 1.11.8:
library(data.table) dt <- data.table(x = rnorm(10), y = rnorm(10)) gs <- list(a = c("x", "y", "z"), b = c("p", "q")) for (g in names(gs)) { dt[, (g) := rowSums(.SD), .SDcols = intersect(colnames(dt), gs[[g]])] }
But if the
for-loop is run again, theRHS ...error occurs.I think I should change my code here.
@renkun-ken With that PR just merged, it should be back to 1.11.8 behavior w.r.t. empty
.SDcols. Please rerun and hopefully this resolves everything? Huge thanks again.Reacted by Kun RenAnother issue: #3245.
Reacted by Matt Dowle@renkun-ken Thanks again, fixed. Please rerun.
Reacted by Kun RenA full run with f73e7b0 works perfectly.
Reacted by Matt DowleReacted by Matt DowleLooks like all issues for milestone 1.12.0 are closed. It's time to initiate a full run.
Reacted by Matt Dowle- CRAN_Release.cmd (e.g. UBSAN, ASAN, rchk)
- GitLab Pipeline (e.g. R 3.1.0, --enable-strict-barrier)
- win-builder 3*OK
- final rerun latest revdeps refreshed (748)
- Kun Ren final go-first completed successfully
It usually takes at least a day or so for CRAN's revdep checks to run plus there's almost-always something unexpected at that stage to iron out and resubmit anyway. So I'll submit to CRAN now so that process can start. If Kun finds anything in his final rerun I can ask CRAN to halt depending on what it is.
- submitted to CRAN
- CRAN pre-test success email received
- CRAN revdep status impact results email received
- Matt replied to confirm and explain differences to submission note.
- Accepted on CRAN
Reacted by Jan Gorecki and Kun Ren@mattdowle Everything works perfectly.
Reacted by Matt DowleReacted by Matt Dowleweb browsers often cache website so might not show new version on cran, the following can be helpful:
Rscript -e 'data.table::fread("https://cran.r-project.org/web/packages/data.table/index.html", sep=NULL, header=FALSE)[24]'
Or Ctrl+F5 does a hard refresh.
I'll close this issue and milestone when I receive the final CRAN email.
CRAN incoming status:
https://cransays.itsalocke.com/articles/dashboard.htmlIn some ways I think the most significant item of this release is note 1. The bug report to r-core is open with no progress. It's potentially really serious and could have affected a lot of packages for a long time, silently.
Like I did with OpenMP for Mac (which worked) the new check and long message is intended to draw attention to it. It asks folk affected to add a comment to the R-core bug report.
If there's any movement on that, I'll be delighted.
We need to gather more very precise and reproducible evidence in the day or so after it hits CRAN and before the Windows binary is made available on CRAN. I believe the most severe problem is only in that short window so we need to gather good evidence right then and there to move the needle in the minds of R-core.
If we can submit a patch then that would help even more. I had a look myself and it looked non-trivial. It likely needs the help of R-core members to explain the logic and comments. And of course testing the patch is non-trivial, time consuming and platform dependent.Sadly, the helpful long new message won't trigger in 1.12.0. Instead there'll be a terse message that
CdllVersion can't be found. That will be improved by #3282 in 1.12.2. At least silent mismatch state wont be possible which is something. For data.table anyway that has this check.Be sure to tweet after cran release will be rolled out so we can get feedback from windows users on that. @mattdowle
Done: https://twitter.com/MattDowle/status/1084528873549705217
Reacted by Matt Dowle1.12.0 is on CRAN.
Windows users please try to install from CRAN having 1.11.8 data.table attached in another R session to verify the mechanism mentioned above to prevent dll version mismatch. As described in #3282 you might seeCdllVersion not foundtype error. You need Rtools installed so R will build Windows binaries from source. Also see @mattdowle's comment above for details.
Hi @renkun-ken.
Could you run latest data.table dev through your systems please.
There are some significant internal code changes that it would be good to run past your checks early.
Please check that more cores are being used more of the time, and whether your test suite total time is any faster or not.
Thanks!!
Matt