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

Rcorr fix #30

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Rcorr fix #30

wants to merge 2 commits into from

Conversation

trljcl
Copy link

@trljcl trljcl commented May 21, 2018

rcorr() returns r values of 1 and p values of 0 for n = 2 comparisons. This leads to many false-positive groupings of pseudospectra represented by sparse EICs. The default should be only to correlate when n = 4 or greater. rcorr() is not catching these because it is counting NA in the EIC matrix in its checks. However, the n values returned by rcorr handily give the number of non-NA comparisons, so code has be updated to filter by a n > 3 threshold, in addition to the the existing r and p thresholds.

xcmsRaw object @scantime updated to match parent xcmsSet object @rt$corrected, where these are present.  This is essential to ensure EICs generated for correlations use the correct rt boundaries
@sneumann
Copy link
Owner

We also need to carefully adapt the currently failing unit tests. The tests simply check for a given result,
which in this case seems to not be the correct result. Yours, Steffen

@sneumann
Copy link
Owner

Hi, the changes look good to me, but any idea why we have
the failures in e.g. https://travis-ci.org/github/sneumann/CAMERA/builds/381696732#L6329 ?
Yours, Steffen

@trljcl
Copy link
Author

trljcl commented Mar 21, 2020 via email

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

Successfully merging this pull request may close these issues.

2 participants