microbiome / benchmarking

Benchmarking miaverse performance
Artistic License 2.0
0 stars 0 forks source link

Introduce analysis on speedyseq #17

Closed RiboRings closed 2 years ago

RiboRings commented 2 years ago

I introduced the melting and agglomeration experiments for speedyseq. The other commands lack an analogue in speedyseq.

Other changes:

antagomir commented 2 years ago

Remarks:

1) Is there a reason to make this PR to "RiboRings" branch, instead of "main" branch? Do the PR against main if possible.

2) Testmethod "agglomerate" gives the following error -TreeSE Error: 'rank' must be a value from 'taxonomyRanks()' -> all other methods tests pass

3) In "Absolute" results, the Asnicar taxa are in lowercase, causing duplicate rows to the panel figure -> Fix.

4) Check that all PDF outputs are ok. For some reason I do not see speedyseq results in beta.pdf, check if this could be fixed.

Otherwise all seems good.

RiboRings commented 2 years ago
  1. I thought this way it would be easier to find bugs before merging into main.
  2. I tried to reproduce it but everything ran smoothly.
  3. I tried to reproduce it but everything ran smoothly.
  4. speedyseq provides alternatives only for psMelt and tax_glom.

May I know which data sets and sample sizes gave the error of point 2?

antagomir commented 2 years ago
  1. Testing can be done in this branch and when the automated checks pass and every reviewer is happy with the PR, it can be merged directly to main. Sometimes projects with multiple developers use more complex hierarchies but then this should be arranged into development layers.

2-3. These are related to the fact that taxonomyRanks(containers[[1]]) is with lowercase and 2-3 with uppercase. All should be comparable (i.e. uppercase). Fix this when reading in the data set.

  1. Ahaa good!