Skip to content
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

plan next release #1084

Closed
5 of 7 tasks
bbolker opened this issue Aug 5, 2024 · 28 comments
Closed
5 of 7 tasks

plan next release #1084

bbolker opened this issue Aug 5, 2024 · 28 comments

Comments

@bbolker
Copy link
Contributor

bbolker commented Aug 5, 2024

Opening this to start planning because it always takes a long time (and because we may get a notice from CRAN any day now [once they return from their vacation] about some of the outstanding CRAN-check issues (which are already fixed in master) ...

@jamesdalg
Copy link

I'm excited by the leverage functions. Hopefully they come out soon. Thanks again.

@strengejacke
Copy link
Contributor

I think #1006 is relatively easy (or already done?). Nice to have would be the second point in #1015.

@mebrooks
Copy link
Contributor

CRAN gave us a deadline for the next release 2024-09-20. They said
"Please see the problems shown on
https://cran.r-project.org/web/checks/check_results_glmmTMB.html.

Specifically, please see the noRemap additional issue.

Compilation fails when using R_CXX_NO_REMAP=true, which compiles C++
code with R_NO_REMAP defined. See chapter "The R API: entry points for
C code" in "Writing R Extensions" for more information.

It is planned that R_CXX_NO_REMAP=true will become the default in due
course, ideally already for R 4.5.0, which in particular makes it
necesssary that all CRAN packages with many strong reverse dependencies
compile/install ok with the new default.

Your package is among the ones with many strong reverse dependencies.
We would thus really appreciate if you could provide a new version of
your package as soon as possible which checks ok with
R_CXX_NO_REMAP=true.

You can verify that your package checks ok with R_CXX_NO_REMAP=true
via R CMD check --as-cran using a current version of R-devel."

@bbolker
Copy link
Contributor Author

bbolker commented Aug 19, 2024

FWIW the R_NO_REMAP stuff was dealt with a long time ago. (From the fix_noremap branch, merged in 4879852.) We just have to decide what we want to squeeze in to the release.

@bbolker
Copy link
Contributor Author

bbolker commented Sep 8, 2024

Our deadline is now 12 days away ... it would be nice to add the experimental leverage stuff from #1065, but at present we can't do it without inducing a dependency on RTMB ... it also fails checks due to use of an unexported function from TMB, and segfaults on GitHub actions (!!)

@bbolker
Copy link
Contributor Author

bbolker commented Sep 17, 2024

Running on win-builder gives a NOTE. This r-pkg-devel post says it is a false positive "now fixed" but maybe it didn't get fixed everywhere? (I think we should mention it in our submission)

I bumped the version and added all the NEWS updates I could find by looking through git logs. As far as I know it's ready to go after reverse dependency checks, although we might want to merge the last 5 unmerged PRs? (#1068, #1075, #1087, #1094, #1097) even though some of them are only partly baked?

@mebrooks
Copy link
Contributor

I have all day tomorrow to focus on this.

@mebrooks
Copy link
Contributor

I get one warning when I check as CRAN on my computer (below). I'm also installing everything on DTU's cluster so I can check the reverse dependencies.

Found the following significant warnings:
  /Library/Frameworks/R.framework/Versions/4.4-arm64/Resources/library/RcppEigen/include/Eigen/src/Core/arch/NEON/PacketMath.h:1671:38: warning: ‘void* memcpy(void*, const void*, size_t)’ copying an object of non-trivial type ‘Eigen::internal::Packet4c’ {aka ‘struct Eigen::internal::eigen_packet_wrapper<int, 2>’} from an array of ‘const int8_t’ {aka ‘const signed char’} [-Wclass-memaccess]
  /Library/Frameworks/R.framework/Versions/4.4-arm64/Resources/library/RcppEigen/include/Eigen/src/Core/arch/NEON/PacketMath.h:1716:38: warning: ‘void* memcpy(void*, const void*, size_t)’ copying an object of non-trivial type ‘Eigen::internal::Packet4c’ {aka ‘struct Eigen::internal::eigen_packet_wrapper<int, 2>’} from an array of ‘const int8_t’ {aka ‘const signed char’} [-Wclass-memaccess]
See ‘/Users/molliebrooks/Documents/glmmTMB development/glmmTMB/glmmTMB.Rcheck/00install.out’ for details.
* used C++ compiler: ‘g++ (GCC) 11.0.0 20201205 (experimental)’

@mebrooks
Copy link
Contributor

mebrooks commented Sep 19, 2024

Since the sun never sets on the glmmTMB development team, I'll go ahead and ask what our gameplan should be for the next 24 hours.

In terms of branches

@mmaechler or @kaskr, any idea about the CRAN warning above that I got but Ben didn't? I also got it on a slightly older version of R today before updating.

Installing everything new on DTU's new cluster is taking longer than expected, so I'm still working on setting up revdep checks .

@bbolker
Copy link
Contributor Author

bbolker commented Sep 20, 2024

I never heard back from Gavin Simpson about #1075; I doubt it would hurt to include it ...

Unfortunately #1067 is a little bit of a mess, I'm trying to clean it up now. I have all the existing tests working now; I will try to do some very basic tests of the intended extended functionality and see how it goes ...

(I just posted a very basic example here. It's rough, but if DHARMa authors would find it useful to have it available (and be willing to update for future releases ...), and if it doesn't break anything else, I think we could include it ...

@mmcgillycuddy
Copy link
Contributor

Hi @mebrooks, I think it's ok to go out for people to trial. I would just like to add something to the vignette so there is an example.

@mebrooks
Copy link
Contributor

Do the GitHub actions automatically check all OS? I've been doing that using rhub for previous CRAN submission, but rhub updated to a new version which is quite different and it sounds like it may be redundant with GitHub actions.

I'm checking the reverse dependencies of the current master branch and I'll rerun them after we merge in #1067. I think it's a safe thing to do because you're just adding a new function.

@bbolker
Copy link
Contributor Author

bbolker commented Sep 20, 2024

I'm getting a little worried now. I have to go to work and teach now and am pretty busy most of the day. I'm getting some test failures in test-propto, not sure why I haven't seen them before now. This looks like an extend_re_form-specific problem though ...

@bbolker
Copy link
Contributor Author

bbolker commented Sep 20, 2024

It seems ridiculous, but I think our choices might be (1) hold off on extend_re_form for now (and make a concerted effort to clean it up and push a new version in a month or so), or (2) ask CRAN for a 24-hour extension ...

@mebrooks
Copy link
Contributor

Ok, I wrote to CRAN for an extension and will keep you updated.

I can't figure out this warning. When I had a similar problem before it was fixed by using CRAN's version of TMB, but that doesn't seem to be working.

I'll keep testing and trying.

@mebrooks
Copy link
Contributor

Kasper says the warning I was getting looks like a problem in Eigen, so there's nothing we can do to fix it:
https://gitlab.com/libeigen/eigen/-/issues/2326
And it would occur on any package that links to RcppEigen.

The reverse dependency checks are still running so I'm going to take a break and come back in an hour. I tried to understand the errors that extend_re_form causes in test-propto.R, but they're burried pretty far down.

@mebrooks
Copy link
Contributor

mebrooks commented Sep 20, 2024 via email

@mebrooks
Copy link
Contributor

mebrooks commented Sep 23, 2024

The only problems with reverse dependencies of the current main branch (extend RE form not yet merged) are ones that we saw in the summer. So I think I just have to write to those maintiners and tell them our plan to submit this week.

UPDATE: I wrote to them.

@bbolker
Copy link
Contributor Author

bbolker commented Sep 23, 2024

I think I've fixed the problems in the extend_re_form branch. @mebrooks are you willing to merge that PR and re-run the reverse dependency check again? (presumably now that you have it set up it will just be more compute time, not a big hassle for you to get it started .. ??)

@mebrooks
Copy link
Contributor

Yeah, the reverse dependency check is not much work now that it's set up. I'm running them now and should have results in the morning.

@mebrooks
Copy link
Contributor

All the revdep and Winbuilder checks are OK. So we just have to decide if we should include #1097 and address #1099. Then of course I'll do the checks again. I have about 3 hours tomorrow to work on this.

@bbolker
Copy link
Contributor Author

bbolker commented Sep 26, 2024

I don't think either #1097 (too preliminary, untested) or #1099 (bug is in reformulas, can be fixed elsewhere, predates this release) are worth worrying about for this release.

You might want to comment that the dotwhisker package, while currently archived, is actively working to get back on CRAN, so should be back shortly.

@mebrooks
Copy link
Contributor

I submitted and told mtscr.

@strengejacke
Copy link
Contributor

In the docs fpr ?nbinom12, that family could be moved to the other nbiom-entries. Currently, nbinom1 and 2 appear at the top of the list, nbinom12 at the end.

@strengejacke
Copy link
Contributor

If you haven't submitted yet - I found a bug, possibly in the tweedie family. Let me file an issue.

@strengejacke
Copy link
Contributor

(ok, just saw, I'm 3 hours late: https://r-hub.github.io/cransays/articles/dashboard.html)

@bbolker
Copy link
Contributor Author

bbolker commented Sep 27, 2024

In the docs fpr ?nbinom12, that family could be moved to the other nbiom-entries. Currently, nbinom1 and 2 appear at the top of the list, nbinom12 at the end.

FWIW I think this has to do with the order that the functions actually appear in the code (since they're all defined with an @rdname nbinom2 roxygen tag ...)

bbolker added a commit that referenced this issue Sep 27, 2024
@bbolker bbolker closed this as completed Sep 27, 2024
@strengejacke
Copy link
Contributor

FWIW I think this has to do with the order that the functions actually appear in the code (since they're all defined with an @Rdname nbinom2 roxygen tag ...)

Yes, I know, but that's probably not what users see/know ;-)

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

No branches or pull requests

5 participants