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

Release 2.5-4 before Feb 8, 2019 #305

Closed
jarioksa opened this issue Jan 26, 2019 · 8 comments
Closed

Release 2.5-4 before Feb 8, 2019 #305

jarioksa opened this issue Jan 26, 2019 · 8 comments
Milestone

Comments

@jarioksa
Copy link
Contributor

jarioksa commented Jan 26, 2019

Kurt Hornik of CRAN sent email saying:

Dear maintainer,

Please see the problems shown on
https://cran.r-project.org/web/checks/check_results_vegan.html.

Please correct before 2019-02-08 to safely retain your package on CRAN.

The breakage is the change of formula in R (issue #299) that we have already patched in the cran-2.5 branch.

There are not so many changes in this release, but this would also fix #303, #304 and the goodness problem that @legendre reported in email. adonis NA handling (issue #302) cannot be merged to this minor release: master branch has changed too much, and it must wait for the 2.6 releases.

I have also flagged #295 as a milestone issue that should be solved for this release. This issue is about handling partial ordination models in RsquareAdj. This is technically easy: we just revert some of the commits that removed Radj2 for partial models. It is only about deciding which commits should be reverted: RDA is obvious, but how to handle CCA?

@jarioksa jarioksa added this to the 2.5-4 milestone Jan 27, 2019
@jarioksa
Copy link
Contributor Author

jarioksa commented Jan 31, 2019

Running win-builder gives an error that I can diagnose but I cannot trigger in my system. It is about if() conditions being longer than length 1. This used to be a warning, but win-builder makes it an error. The problem is that I cannot turn this into an error in my system and I cannot detect these cases and correct them. I have set R environment variable _R_CHECK_LENGTH_1_CONDITION_='TRUE' (with TRUE alternatively lower case, with or without quotes). This sets the environment OK when I use vector conditions in command line (such as if(runif(10) > 0.5) TRUE): This gives an error like should with this setting. However, when I run example(cca) with this problem, everything goes smoothly, and so goes R CMD check.

This error is not reported in the CRAN check page https://cran.r-project.org/web/checks/check_results_vegan.html, but I think CRAN submission check will find this. Therefore this should be fixed before the release. The only way I can proceed is to fix the check error win-builder reports, make a new package, submit it to win-builder, see where it fails and loop again. So if @gavinsimpson @psolymos or anybody can trigger the error, please have a look a the fixes (which usually are very simple after you find them). If you try checking these issues, remember to git fetch the current vegan tree, where I have fixed two cases.

Here an excerpt from the win-builder test which comes from example(cca):

* checking examples ...
** running examples for arch 'i386' ... ERROR
Running examples in 'vegan-Ex.R' failed
The error most likely occurred in:

> base::assign(".ptime", proc.time(), pos = "CheckExEnv")
> ### Name: cca
> ### Title: [Partial] [Constrained] Correspondence Analysis and Redundancy
> ###   Analysis
> ### Aliases: cca cca.default cca.formula rda rda.default rda.formula
> ### Keywords: multivariate
> 
> ### ** Examples
> 
> data(varespec)
> data(varechem)
> ## Common but bad way: use all variables you happen to have in your
> ## environmental data matrix
> vare.cca <- cca(varespec, varechem)
> vare.cca
Call: cca(X = varespec, Y = varechem)

              Inertia Proportion Rank
Total          2.0832     1.0000     
Constrained    1.4415     0.6920   14
Unconstrained  0.6417     0.3080    9
Inertia is scaled Chi-square 

Eigenvalues for constrained axes:
  CCA1   CCA2   CCA3   CCA4   CCA5   CCA6   CCA7   CCA8   CCA9  CCA10  CCA11 
0.4389 0.2918 0.1628 0.1421 0.1180 0.0890 0.0703 0.0584 0.0311 0.0133 0.0084 
 CCA12  CCA13  CCA14 
0.0065 0.0062 0.0047 

Eigenvalues for unconstrained axes:
    CA1     CA2     CA3     CA4     CA5     CA6     CA7     CA8     CA9 
0.19776 0.14193 0.10117 0.07079 0.05330 0.03330 0.01887 0.01510 0.00949 

> plot(vare.cca)
> ## Formula interface and a better model
> vare.cca <- cca(varespec ~ Al + P*(K + Baresoil), data=varechem)
> vare.cca
Call: cca(formula = varespec ~ Al + P * (K + Baresoil), data =
varechem)

              Inertia Proportion Rank
Total           2.083      1.000     
Constrained     1.046      0.502    6
Unconstrained   1.038      0.498   17
Inertia is scaled Chi-square 

Eigenvalues for constrained axes:
  CCA1   CCA2   CCA3   CCA4   CCA5   CCA6 
0.3756 0.2342 0.1407 0.1323 0.1068 0.0561 

Eigenvalues for unconstrained axes:
    CA1     CA2     CA3     CA4     CA5     CA6     CA7     CA8 
0.27577 0.15411 0.13536 0.11803 0.08887 0.05511 0.04919 0.03781 
(Showing 8 of 17 unconstrained eigenvalues)

> plot(vare.cca)
> ## Partialling out and negative components of variance
> cca(varespec ~ Ca, varechem)
Call: cca(formula = varespec ~ Ca, data = varechem)

              Inertia Proportion Rank
Total         2.08320    1.00000     
Constrained   0.15722    0.07547    1
Unconstrained 1.92598    0.92453   22
Inertia is scaled Chi-square 

Eigenvalues for constrained axes:
   CCA1 
0.15722 

Eigenvalues for unconstrained axes:
   CA1    CA2    CA3    CA4    CA5    CA6    CA7    CA8 
0.4745 0.2939 0.2140 0.1954 0.1748 0.1171 0.1121 0.0880 
(Showing 8 of 22 unconstrained eigenvalues)

> cca(varespec ~ Ca + Condition(pH), varechem)
Call: cca(formula = varespec ~ Ca + Condition(pH), data = varechem)

              Inertia Proportion Rank
Total          2.0832     1.0000     
Conditional    0.1458     0.0700    1
Constrained    0.1827     0.0877    1
Unconstrained  1.7547     0.8423   21
Inertia is scaled Chi-square 

Eigenvalues for constrained axes:
   CCA1 
0.18269 

Eigenvalues for unconstrained axes:
   CA1    CA2    CA3    CA4    CA5    CA6    CA7    CA8 
0.3834 0.2749 0.2123 0.1760 0.1701 0.1161 0.1089 0.0880 
(Showing 8 of 21 unconstrained eigenvalues)

> ## RDA
> data(dune)
> data(dune.env)
> dune.Manure <- rda(dune ~ Manure, dune.env)
> plot(dune.Manure) 
 ----------- FAILURE REPORT -------------- 
 --- failure: length > 1 in coercion to logical ---
 --- srcref --- 
: 
 --- package (from environment) --- 
vegan
 --- call from context --- 
plot.cca(dune.Manure)
 --- call from argument --- 
!is.null(g$centroids) && !is.na(g$centroids)
 --- R stacktrace ---
where 1: plot.cca(dune.Manure)
where 2: NextMethod("plot", x, ...)
where 3: plot.rda(dune.Manure)
where 4: plot(dune.Manure)

 --- value of length: 10 type: logical ---
        RDA1 RDA2
Manure0 TRUE TRUE
Manure1 TRUE TRUE
Manure2 TRUE TRUE
Manure3 TRUE TRUE
Manure4 TRUE TRUE

@psolymos
Copy link
Contributor

psolymos commented Jan 31, 2019

I couldn't trigger the error, but it looks like we have is.na(g$centroids)[1] at one place and is.na(g$centroids) at 3 more places in plot.cca. The report suggests that the second case (or all cases for that matter) should be any(is.na(g$centroids)) (or all(is.na(g$centroids)) when it is negated) to avoid having length >1 in if condition.

@eduardszoecs
Copy link
Contributor

Can you set options(warn=2, error=recover) to turn the warnings into errors & start the debugger at warning/error?

@jarioksa
Copy link
Contributor Author

jarioksa commented Feb 2, 2019

@Edild the "problem" is that the function does not even give a warning in Linux or in Mac, but triggers an error in Windows. We also have a continuing checking in github both in Linux and in Windows, but these pass. There is something in the win-builder that I cannot reproduce, and mere setting of _R_CHECK_LENGTH_1_CONDITION_ does not help.

If I look at the internal code, I see that in example(cca) and there in plot(dune.Manure) the g$centroids returns length > 1:

> is.na(g$centroids)
         RDA1  RDA2
Manure0 FALSE FALSE
Manure1 FALSE FALSE
Manure2 FALSE FALSE
Manure3 FALSE FALSE
Manure4 FALSE FALSE

However, in Mac (and obviously in Linux, but I have no Linux at home) the whole condition reduces to length 1:

> length(g$centroids) > 0 && is.na(g$centroids)
[1] FALSE

It seems that this is not reduced to length 1 in win-builder or the checking happens in some wilder way.

@psolymos : the fix is indeed simple, like you said. It suffices to wrap the condition within all() or any() or explicitly indexing with [1L], but the problem is finding these places when I don't get an error. Of course I can go on with win-builder checks, but it sounds wasteful: the first error came from adonis, the second from cca and there are still help pages cca.object to wcmdscale to go in alphabetic order.

@jarioksa
Copy link
Contributor Author

jarioksa commented Feb 2, 2019

@Edild @psolymos I think I have solved the problem of testing, but cannot test this at home: R-devel has a new environment variable (https://cran.r-project.org/doc/manuals/r-devel/R-ints.html):

_R_CHECK_LENGTH_1_LOGIC2_
Optionally check if either argument of the binary operators && and || has length greater than one. The format is the same as for _R_CHECK_LENGTH_1_CONDITION_. Default: unset (nothing is reported)

This seems not be ported to release versions, and I don't have R-devel in my tiny laptop so that setting this variable does nothing. R-devel NEWS gives this as an "experimental" feature, but that does not mean that CRAN incoming check & the CRAN team would be forgiving here.

The length > 1 conditions has triggered a warning for ages, and we have fixed all those cases. So the remaining cases are just violations or _R_CHECK_LENGTH_1_LOGIC2_. If any of you has R-devel and a plenty of free time, you are free to find these violating pieces of code and fix them (@gavinsimpson ?).

I retired yesterday so that my access to a Linux computer w/R-devel at the uni is more limited now.

jarioksa pushed a commit that referenced this issue Feb 2, 2019
The test is run with environment variable _R_CHECK_LENGTH_1_LOGIC2
in R-devel (to be R 3.6.0), and this made vegan fail in
winbuilder checks for R-devel.

See issue #305 in github.
jarioksa pushed a commit that referenced this issue Feb 2, 2019
Yet another issue of _R_CHECK_LENTH_1_LOGIC2_ issues (github #305).
lpolygon() worked correctly with old code, because only the first
of possible values col, lwd, border etc was used, but strict testing
of equal lenghts of elements in && and || gave an obscure error.
jarioksa pushed a commit that referenced this issue Feb 2, 2019
The test is run with environment variable _R_CHECK_LENGTH_1_LOGIC2
in R-devel (to be R 3.6.0), and this made vegan fail in
winbuilder checks for R-devel.

See issue #305 in github.

(cherry picked from commit e6b37b4)
jarioksa pushed a commit that referenced this issue Feb 2, 2019
Yet another issue of _R_CHECK_LENTH_1_LOGIC2_ issues (github #305).
lpolygon() worked correctly with old code, because only the first
of possible values col, lwd, border etc was used, but strict testing
of equal lenghts of elements in && and || gave an obscure error.

(cherry picked from commit bb2d019)
@jarioksa
Copy link
Contributor Author

jarioksa commented Feb 2, 2019

I said I wouldn't, but I built R-devel in my Macbook Air. This time I was more persistent and chased the libraries needed for building R. With this, I could find and fix the _R_CHECK_LENGTH_1_LOGIC2_ cases, and now vegan 2.5-4 passes win-builder tests. (Most fixes were obvious, but in ordixyplot the error was deep down in the grid package that provides the infrastructure for lattice, and I was lost for a long time).

This means that I can submit 2.5-4 on Sunday Feb 3 or the following Monday leaving some buffer for the Feb 8 deadline.

@legendre
Copy link
Contributor

legendre commented Feb 2, 2019 via email

@jarioksa
Copy link
Contributor Author

jarioksa commented Feb 4, 2019

vegan 2.5-4 was released today with these fixes.

@jarioksa jarioksa closed this as completed Feb 4, 2019
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

4 participants