theislab / scCODA

A Bayesian model for compositional single-cell data analysis
BSD 3-Clause "New" or "Revised" License
147 stars 24 forks source link

Error when num_burnins > num_results #79

Closed dawe closed 1 year ago

dawe commented 1 year ago

Hi all, I've found (by accident) that model.sample_hmc(num_burnin=X, num_results=Y) crashes if X > Y:

File ~/miniforge3/envs/may23/lib/python3.10/site-packages/sccoda/model/scCODA_model.py:816, in scCODAModel.get_y_hat(self, states_burnin, num_results, num_burnin)
    812 b_raw_ = np.einsum("...jk, ...jl->...jk", b_offset, sigma_d)
    814 beta_temp = np.einsum("..., ...", ind_, b_raw_)
--> 816 beta_ = np.zeros(chain_size_beta)
    817 for i in range(num_results - num_burnin):
    818     beta_[i] = np.concatenate([beta_temp[i, :, :self.reference_cell_type],
    819                                np.zeros(shape=[self.D, 1], dtype=np.float64),
    820                                beta_temp[i, :, self.reference_cell_type:]], axis=1)

ValueError: negative dimensions are not allowed
johannesostner commented 1 year ago

Hi! Thanks for the find! Since num_results is the number of all samples (burn-in + posterior samples), this is expected because X>Y would mean a negative number of posterior samples. We should catch that before running the sampling process, though. In our pertpy implementation, we don't have this problem, as it expects the number of posterior samples as an input.

dawe commented 1 year ago

Nice, thanks! One question, as pertpy implements CODA and, as far as I can see, the same kind of analysis (and more), is it recommended to switch to it? Are scCODA and pertpy being both developed/updated?

johannesostner commented 1 year ago

As stated in the readme of this repository, pertpy will be the only maintained implementation of the scCODA model. We will not regularly update the scCODA package anymore, and it is recommended to switch to pertpy

dawe commented 1 year ago

The good old RTFM!