Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Mixed usage of n_samples_overall in get_normalized_expression #2641

Open
martinkim0 opened this issue Apr 1, 2024 · 2 comments
Open

Mixed usage of n_samples_overall in get_normalized_expression #2641

martinkim0 opened this issue Apr 1, 2024 · 2 comments
Labels

Comments

@martinkim0
Copy link
Contributor

martinkim0 commented Apr 1, 2024

In TOTALVI.get_normalized_expression, MULTIVI.get_normalized_expression, and ContrastiveVI.get_normalized_expression, n_samples_overall just downsamples indices, while in RNASeqMixin.get_normalized_expression, it's used for the number of MC inference samples. This might be confusing for users who expect the argument to mean and do the same thing across methods.

@canergen
Copy link
Member

canergen commented Aug 8, 2024

@PierreBoyeau Currently this means that these other function don't use importance weighting? I assume it would make sense to add it to these models as well?

@canergen canergen removed the backlog label Aug 8, 2024
@canergen
Copy link
Member

canergen commented Aug 8, 2024

I think we can improve the naming as well (but that's styling and I would stick to n_samples_overall for number of mc_samples and n_samples to number of samples for a single input cell in a forward pass.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants