Closed benjjneb closed 4 years ago
Working on this today, @benjjneb sorry for the excessive delay.
Hey @benjjneb - no rush to reply, but are there any updates on the denoise-pyro discrepancies? We are about 2 weeks out from the next scheduled QIIME 2 release, and trying to see what outstanding PRs might make the release. Do you think we should include this PR, or defer to the next release (tentatively August)?
I am going to look at this tomorrow. If I can figure it out then, I'd be in favor of getting it in this release. Some way to get singleton ASVs seems like a nice thing to get out to Q2 users.
There's no reason to delay other than my time being a roadblock.
Great, thanks for the update, @benjjneb. Let me know what we can do on our end, when you know it, and we'll be there.
The denoise-pyro
bug that caused the change in test results should be fixed by e91ee9403195c92270fd9635ee7a711a8d19ea1c
This pull request adds pseudo-pooling to all DADA2 workflows: denoise-single, denoise-pyro, and denoise-paired. It is implemented in a way that should keep memory costs flat, and barely higher than they are when not using pseudo-pooling.
Pseudo-pooling is activated by the new parameter
pooling-method = pseudo
. It's main effect is to increase sensitivity to rare variants in samples, particularly singletons, as long as they were observed in other samples independently.This passes testing on my end based on Rscript invocations, but python based testing still hasn't been performed, and no new pseudo-pooling specific tests have been added.
This addresses https://github.com/qiime2/q2-dada2/issues/87 but I haven't added full pooling yet (which is much more memory/time intensive).
I'm not sure how to deal with the unit-tests and python testing. In principle, the current tests should all pass as the output using the default
pooling-method = independent
should be unchanged from the previous output.I realize this is probably too late for the upcoming release.