Conversation
ilia-kats
commented
Aug 14, 2025
ilia-kats
commented
- make it match the R reference implementation
- implement some options that are present in the R reference implementations but were missing here
ilan-gold
left a comment
ilan-gold
left a comment
There was a problem hiding this comment.
Small things, but nice1
|
|
||
| if denoise_counts: | ||
| bgmeans = np.empty(cells_scaled.shape[0], np.float32) | ||
| bgmeans = np.empty(cells_scaled.shape[0], cells_scaled.dtype) |
There was a problem hiding this comment.
Why are we making the bgmeans the same dtype as the input? It looks like bgmeans is filled in with mean values - why not just set it to float64? It seems like you'd have the same type-loss problem as with ints then
There was a problem hiding this comment.
At that point, cells_scaled will always be a float. If it's float32, then the GaussianMixture results will also be float32, so no need to waste memory by using a dtype that is too large.
There was a problem hiding this comment.
Alternatively, I can move the castback to the very bottom of the function, so we do all our calculations in float64 and only cast back the result if the input was float32.
|
|
||
| if denoise_counts: | ||
| bgmeans = np.empty(cells_scaled.shape[0], np.float32) | ||
| bgmeans = np.empty(cells_scaled.shape[0], cells_scaled.dtype) |
2 participants
