Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
Already on GitHub? Sign in to your account
save dependent data matrix only once in constrained ordination object #227
Comments
jarioksa
added the
request-for-comments
label
Feb 16, 2017
jarioksa
added a commit
that referenced
this issue
Feb 22, 2017
|
|
jarioksa |
ba860a5
|
|
I have opened a new branch ordination-Xbar-issue-#227 which implements this change. At the first stage, I have
The current function allows using updated functions both with the newly created and legacy result objects. Currently the legacy result objects are handled silently and my plan is to handle them silently in the first release (2.5-0). In later releases, the old style functions are handled with a warning that urges users to update the objects, and finally we remove the compatibility function completely. In most cases the old object |
jarioksa
referenced this issue
Feb 28, 2017
Merged
Change in CCA object structure (saving Xbar, issue #227) #228
|
The suggested change will be made in PR #228. This is the first step, and still keeps the old structure so that now the working data may be saved in four versions. The removal of PR #228 provides function My plan is to have the new structure in vegan 2.5-0 (with no scheduled release date). I think we need to keep the old superfluous (and large!) items till other functions can adapt to the change, although vegan won't touch those items any longer. There will be legacy result objects around for a long time (i.e., created with previous versions of vegan and saved in the workspace), but
Comments? |
jarioksa
added a commit
that referenced
this issue
Mar 2, 2017
|
|
jarioksa |
b24fa6a
|
|
Merging of PR #228 closes the implementation part of this issue. The transition strategy part is still open. |
jarioksa
added a commit
that referenced
this issue
Mar 20, 2017
|
|
jarioksa |
f831a5b
|
|
Function |
jarioksa
added this to the
2.5-0
milestone
May 17, 2017
|
TODO: run package tests against dependent packages to find the maintainers of dependent packages that need to be pre-alerted for the change. |
jarioksa commentedFeb 16, 2017
•
edited
I suggest to simplify the internal structure of
"cca"object by saving the analysed dependent data only once. Currently we have up to three versions of the same data in the ordination objectx:x$pCCA$Fit: fitted values for partial termsx$CCA$Xbar: residual values after partial termsx$CA$Xbar: residual values after partial terms and constraintsAll these matrices can be found from the original matrix with appropriate QR decomposition item using
qr.fitted()andqr.resid(). We do it already now: very often we need the fitted values after constraints (and conditions) that we have not saved, but we need to find it asqr.fitted(x$CCA$QR, x$CCA$Xbar). Using QR decomposition is pretty cheap: we do it several times in each permutation ofanova.cca&permutest.ccaalready now. So we could have considerable saving of result object sizes with little cost on time by saving only the original adjusted matrix and using QR operations when needed. Currently that transformed dependent matrix is returned byinitXXXX()functions inR/ordConstrained.R.A more severe problem is that we sometimes need the original transformed matrix prior to any partial terms or constraints. In most cases we can get that as
x$pCCA$Fit + x$CCA$Xbar, but that does not work fordbrda(), and therefore several must be disabled for partial db-RDA. Saving the original input data (afterinitDBRDA()) would solve all these problems.The only problem that I see with this is that the
"cca"class result object will change, and this means that several support functions need to be changed. We also need to check that this does not break other packages using vegan, and alert their maintainers if necessary. The current"cca"structure is not a result of very thorough consideration: I just happened to writecca.default()this way in 2002, and since then other functions have been adopted to this structure.In vegan the following functions must be adopted to this suggested change:
fittedmethodsgoodness.cca(now enabled for partialdbrda)inertcomp(now enabled fordbrda)mso(never implemented fordbrda)permutest.cca(model = "direct"enabled for partialdbrda)predict.rdasimulatemethods (not implemented fordbrda)stressplotmethods (now enabled for partialdbrda)tolerance.cca(only meaningful forcca)