Initialise agglomerateByModule

[RiboRings]

Basic agglomerateByModule function leveraging cross-product was initialised for SummarizedExperiment class.

Consider whether we should:

• Add separate method for TreeSE
• Add support for sample modules in colData
• Integrate more with existing agglomerate and merge functions

[RiboRings]

Example usage:

devtools::load_all()

data("GlobalPatterns")
tse <- GlobalPatterns

# Add some random modules
N_module <- 3L
modules <- sample(c(TRUE, FALSE),
                  size = nrow(tse) * N_module,
                  prob = c(0.2, 0.8),
                  replace = TRUE)

modules <- modules |> matrix(nrow = nrow(tse))
colnames(modules) <- paste0("module_", seq_len(ncol(modules)))
rowData(tse) <- cbind(rowData(tse), modules)

# Extract module columns
module_columns <- grep("module_", colnames(rowData(tse)), value = TRUE)

# Agglomerate based on modules
tse_module <- agglomerateByModule(tse, by = 1, group = module_columns)

[antagomir]

Seems good. Some notes:

**Add separate method for TreeSE: ** this is feasible for agglomerateByRanks where the grouping can follow phylogenetic tree. For modules the tree aggomeration might be ill-defined in many cases? Is it meaningful to do that for modules in the general case (or are there relevant special cases to support?). This can be added later if this requires more thinking.

**Add support for sample modules in colData: ** ideally this method could be symmetric and could be applied for rows and cols alike. We also have other such functions and that should be the default.

Integrate more with existing agglomerate and merge functions: Is there a fundamental difference compared to agglomerateByVariable? If not, we might like to combine these two. It might currently operate only with one column at the time but that is minor difference and just a special case of multi-column thing.

[RiboRings]

Add separate method for TreeSE I agree with you, so it seems preferable to downgrade TreeSE to SE and remove trees when agglomerating by module.

Add support for sample modules in colData: Now it works for both features and samples.

Integrate more with existing agglomerate and merge functions: I came to notice a subtle difference between the two. While agglomerateByVariable takes factor variables as groups, agglomerateByModule requires one or more binary (0/1, F/T) variables. Therefore, the two would be analogous only if factor variables were first one-hot encoded and used to agglomerate, but this does not sound like a convenient solution for factors with many levels. So in general, I would keep them as they are (partially integrated) and recommend agglomerateByVariable for single factor groups and agglomerateByModule for one or multiple logical groups.

[RiboRings]

This PR is ready for review

[TuomasBorman]

Looks good!

I guess this is more efficient than implementation in https://github.com/microbiome/mia/tree/agglomerateByModule (i.e., the branch can be removed)?

[antagomir]

Still on this point:

Integrate more with existing agglomerate and merge functions:

• agglomerateByVariable takes factor variables as groups
• agglomerateByModule requires one or more binary (0/1, F/T) variables

Your argument was that factors with multiple levels should be one-hot-encoded to use the same.

But on the other hand, isn't it possible to use agglomerateByVariable for binary variables, and then it will essentially reduce to the same than agglomerateByModule? If so, agglomerateByModule would not have added value because same could be obtained with agglomerateByVariable applied on binary variables/factors/logicals/numerics.

But am I missing sometihing?

[RiboRings]

Several binary modules can be reduced to a single factor column as long as each feature belongs to only one module. Otherwise, the module matrix is necessary, in which case using agglomerateByVariable for each module would return one agglomerated SE with two rows (module = TRUE and module = FALSE) for each and every module, whereas agglomerateByModule would return one agglomerated SE with all modules as rows.

[antagomir]

Yes indeed, unless we consider the case where the variable in agglomerateByVariable contains NAs, which will not be agglomerated. Binary module encoded with NA + TRUE could result in a single row per module. This is more unclear way to implement something like that.

Another point is that agglomerateByVariable has been written to collapse rows (or cols) based on a single variable (e.g. factor with levels). It could be extended to cover case with multiple variables but this would more intuitively be a set of collapsed matrices with varying dimensionality (which would be determined based on the number of factor levels per each variable). Matrices with varying dimensionality could go into altExps. This could make sense in principle but it would indeed be a different method than agglomerateByModule.

[RiboRings]

This PR is ready for final check/merging. Now the modules are retrieved from a metadata slot specified in groups argument.

[antagomir]

Seems good, just some suggestions.

Checks still fail?

[antagomir]

I let it for you guys to see when this should be merged.

[TuomasBorman]

Thanks!

The code seems ready. Only minor things (see comment). I will still check Zulip discussion.

There has been discussion in many channels. Why metadata slot was chosen over row/colData? Adding module information to rowData would be more intuitive in my opinion as there should be module information for every row(?) and allow tracking the rows.

This would also simplify the method as we can expect that the module information is in specific format.

[RiboRings]

The code seems ready. Only minor things (see comment). I will still check Zulip discussion.

Ok. I'll open a separate PR for the function that adds modules from (BugSigDB) signatures. After that I think we can add a more concrete example to the vignettes.

There has been discussion in many channels. Why metadata slot was chosen over row/colData? Adding module information to rowData would be more intuitive in my opinion as there should be module information for every row(?) and allow tracking the rows.

This would also simplify the method as we can expect that the module information is in specific format.

I agree that rowData could be a more intuitive location. After our Zulip discussion, we concluded that putting modules in a separate slot could be more convenient/efficient as the number of modules can easily get very big.

If things go in the direction I think, users won't really need to manually add modules to either metadata or rowData, because the modules table will be internally derived from the signatures output by a database (like in the case of BugSigDB). That way, we are free to decide where is best to store modules.

[RiboRings]

Ready to be checked and merged.

[antagomir]

Seems good for now.

In this paper (appendix, p. 12), we have a list of butyrate producing bacteria:

This is a realistic use case for the module method.

Two observations:

1. The module is often in practice tied to a particular preprocessing method (e.g. Greengenes2, MetaPhlAn 2 vs 3 vs 4...).
2. Real modules may contain features from different levels of the row hierarchy (in the above case, mostly genus but also one species, because the genus would be too generic)

Suggestion:

• Let us think whether a "module" could include alternative versions / mappings for different processing methods. For instance, a module could be a named list, each element corresponding to one processing method. If not provided then the method could assume that there is just one version that directly applies. Or the versions could be in the original module definition table if there is then a handy R importer that can also pick the selected version. Note that also BugSigDB supports this kind of thing (ncbi, metaphlan..)
• consider how to support cases including multiple feature levels, and show that in the examples
• include the above case as a tiny demo data set in the package and use that in the examples? This would be directly useful in subsequent publications using this feature.

[TuomasBorman]

In agglomeration-point-of-view, storing these modules to rowData would be a simpler approach. As Leo mentioned, modules include usually taxonomy information --> rowData would be ideal place to store the module

Moreover, if modules are stored to metadata(), they cannot be used for instance in visualization.

Instead of making complex agglomeration function for different module-formats, it could rely that the module information is in rowData. Specific importers could make sure that the module data is wrangled into correct format.

[antagomir]

If it is too difficult, we can first provide a simpler method that does not consider multiple (e.g. taxonomic) ranks.

But this would be relevant immediate extension as the example shows.

[RiboRings]

Ok now it's back to row/colData.

[RiboRings]

If it is too difficult, we can first provide a simpler method that does not consider multiple (e.g. taxonomic) ranks.

But this would be relevant immediate extension as the example shows.

Thanks for rising this point!

I prefer addressing this issue in the next PR because the solution might also depend on the function to convert BugSigDB metaphlan signature to modules table. Because we probably want to consider mixed ranks already when making the modules table.

For modules with mixed ranks, we can check how the bugsigdbr function getSignatures is doing it (and maybe other useful functionality).

[antagomir]

Perfect. Make sure we don't reinvent the feel w.r.t BugSigDB if they already developed similar solutions.

Can you open a new issue on the multirank thing, so we keep track on it?

[TuomasBorman]

Looks good, thanks!