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

goodness.cca currently ignores choices argument #190

Closed
gavinsimpson opened this issue Aug 17, 2016 · 6 comments
Closed

goodness.cca currently ignores choices argument #190

gavinsimpson opened this issue Aug 17, 2016 · 6 comments
Labels
Milestone

Comments

@gavinsimpson
Copy link
Contributor

As mentioned in the discussion surrounding #189, the choices argument it not used in goodness.cca.

library(vegan)
data(dune, dune.env)
goodness(cca(dune ~ A1 + Moisture + Manure, dune.env), choices = 2)
> head(goodness(cca(dune ~ A1 + Moisture + Manure, dune.env), choices = 2))
               CCA1      CCA2      CCA3      CCA4      CCA5      CCA6      CCA7
Achimill 0.33914782 0.4686996 0.5161329 0.5627598 0.6260376 0.6277621 0.6349226
Agrostol 0.47902756 0.6854956 0.6866681 0.6921161 0.7609479 0.7684217 0.7697606
Airaprae 0.03895767 0.2844500 0.3369202 0.4081454 0.4326753 0.4396726 0.4413921
Alopgeni 0.04957140 0.5630997 0.5761448 0.7128628 0.7331946 0.7332199 0.7334576
Anthodor 0.04059179 0.3099049 0.3245315 0.4196064 0.4915799 0.4945911 0.5095028
Bellpere 0.23991007 0.2612923 0.2612967 0.2715557 0.3876347 0.4673342 0.4697798
              CCA8
Achimill 0.6405579
Agrostol 0.7765521
Airaprae 0.4510074
Alopgeni 0.7341331
Anthodor 0.5096177
Bellpere 0.4700591

It seems that in the transition from the old goodness.cca code to the one that was goodness2.cca this functionality got lost. Originally we had:

    if (!missing(choices)) 
        vexp <- vexp[, choices, drop = FALSE]

but this is not in the newer (current) version for the code.

This is clearly a bug, but do we want to return the original choices functionality or update the documentation.

If we reinstate the functionality, it seems we need a better check that just not being missing; in the example in #189 there is only a single constrained axis so asking for choices = 1:3 doesn't make sense.

@gavinsimpson gavinsimpson changed the title goodness.cca currently ignores choices argument goodness.cca currently ignores choices argument Aug 17, 2016
@sjankin
Copy link

sjankin commented Aug 17, 2016

Referencing this issue and example in #189, any reason why you cannot have contributions to unconstrained axes? That is, beyond CCA choices.

@gavinsimpson
Copy link
Contributor Author

@smikhaylov Maybe I wasn't thinking this through clearly enough... I suppose it makes no difference whether the particular linear combination is constrained or not when computing this, but the current code separates them into CCA and CA axes. In your case with method = "CCA", there is only one column of goodness of fit stats and in that case we'd need to handle a user specification of choices = 1:3 better than what was in the original code.

This is not to say that we couldn't cbind on unconstrained axes so that we had goodness stats for the selected axes, but this is easy for the user to do as well by calling goodness twice.

@sjankin
Copy link

sjankin commented Aug 17, 2016

I think either way would work fine. method argument could be more hardwired, it's not really reacting to choices specification right now.

@jarioksa
Copy link
Contributor

jarioksa commented Aug 17, 2016

Dropping choices looks accidental. Probably we should reinstate the functionality. It is easy to imagine cases where this is useful: think about hundreds of unconstrained axes, or the desire to see how well a 2D plot represents each point.

There are numerous examples how this has been done in vegan. Mere !missing(choices) is not enough, but you need to check that choices do not exceed rank. In summary.cca and stressplot we first cbind components and then apply choices or rank (k in stressplot). This would call for a radical-ish re-design. In some other functions we remain within components (and always check for the existence of choices before applying them -- or fix this as a bug like in e804435 earlier today).

jarioksa pushed a commit that referenced this issue Aug 18, 2016
choices were unintentionally dropped when goodness.cca was replaced
with the new code (then goodness2.cca) in a231931. They are still
documented but won't work.

There are many ways of doing this as discussed in issue #190 in github
@jarioksa
Copy link
Contributor

jarioksa commented Aug 18, 2016

I opened up branch issue-#190 in vegandevs/vegan to put back choices. Please work further with that branch so that we avoid too many loose ends in the repository. Commit 4026ba4 offers one solution to the issue. It seems there are many alternative ways of implementing choices. In normal usage they are all equal, but there are differences in corner cases:

  • What should happen with choices = 3:1? Current commit reverses the axes and gives cumulation from 3 to 1. This sounds weird, but it was the user command.
  • What should happen with choices = 2:3? Current commit will ignore axis 1 and give cumulation only for axes 2 and 3.
  • What should happen with choices = 2:3 and addprevious = TRUE? Current commit will add previous components but still ignore axis 1 of this component.
  • What should happen with choices = 1:2 and summarize = TRUE? Current commit will give the summary (total) only for chosen two axes.

Some of these choices do differ from the pre-2.4-0 version. In old goodness.cca we first estimated cumulative row sums and then subset choices. The current fix would first subset and then get cumulative sums. In old goodness reversed or shuffled axes had original accumulation and the shuffling did not influence the values. The previous axes of a component were also always included in the row-wise accumulation, even whey they were not among choices. Finally, summary was the value of the last axis and was wrong if choices were shuffled so that last one did not give the most complete accumulation.

Both in the old and in the new proposed code, subsetting was only within the component and would not extend to, say, from constrained to unconstrained if choices exceeded constrained rank. To achieve that you must first ask for full constrained choices and then remaining unconstrained choices with addprevious = TRUE.

@sjankin
Copy link

sjankin commented Aug 19, 2016

I think it makes sense. Probably just needs adjusting documentation now. The use of model argument is now required if you go beyond constrained dimensions. A couple example to illustrate (using data from #189).

devtools::install_github("vegandevs/vegan", ref = "4026ba4")
library(vegan)
words <- read_csv("words.csv")
env <- read_csv("env.csv")
cca <- cca(words ~ Idealpoint, data = env)

goodness(cca) #contribution to inertia on CCA1
goodness(cca, choices = 1:3) #produces only contribution to inertia on CCA1
goodness(cca, model = c("CCA", "CA"), choices=1:3) #produces only contribution to inertia on CCA1

#so to go for unconstrained axes you need to specify `model`
goodness(cca, model =  "CA", choices=1:3)

#not specifying the `model` results in a warning message and null vector of results
goodness(cca, choices=2:3)
Warning message:
In sweep(CA, 1, tot, "/") :
  STATS is longer than the extent of 'dim(x)[MARGIN]'

So I think it works fine, just documentation needs to be adjusted, particularly about the model argument for unconstrained choices.

@jarioksa jarioksa modified the milestones: 2.3-1, 2.4-1 Aug 22, 2016
jarioksa pushed a commit that referenced this issue Aug 23, 2016
jarioksa pushed a commit that referenced this issue Aug 30, 2016
choices were unintentionally dropped when goodness.cca was replaced
with the new code (then goodness2.cca) in a231931. They are still
documented but won't work.

There are many ways of doing this as discussed in issue #190 in github

(cherry picked from commit 4026ba4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants