Hvg seurat v3 numba kernel

[Intron7]

Adds a numba kernel for seurat_v3 for sparse matrices. This kernel is a lot faster and more memory efficient. I doesn't copy nor promotes the matrix to 64-bit floats.

[scverse-benchmark]

Benchmark changes

Change	Before [3ba3f46]	After [277c1bf]	Ratio	Benchmark (Parameter)
-	9.09±1ms	6.90±0.06ms	0.76	preprocessing_counts.time_log1p('pbmc3k')

Comparison: https://github.com/scverse/scanpy/compare/3ba3f46b4e6e77e8c6f0551db9663822097b486a..277c1bfb0885234aa757d0fdaeaa9103eb8568e2
Last changed: Thu, 23 May 2024 12:59:15 +0000

More details: https://github.com/scverse/scanpy/pull/3017/checks?check_run_id=25329779518

[codecov]

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 75.86%. Comparing base (3ba3f46) to head (277c1bf).
Report is 47 commits behind head on main.

Files with missing lines	Patch %	Lines
scanpy/preprocessing/_highly_variable_genes.py	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3017      +/-   ##
==========================================
- Coverage   75.87%   75.86%   -0.01%     
==========================================
  Files         110      110              
  Lines       12533    12533              
==========================================
- Hits         9509     9508       -1     
- Misses       3024     3025       +1     

Files with missing lines	Coverage Δ
scanpy/preprocessing/_highly_variable_genes.py	95.20% <85.71%> (-0.40%)	⬇️

[Intron7]

~500k Cells.
New Version:
peak memory: 16117.61 MiB, increment: 7709.86 MiB
Wall time: 11 s
Old Version:
peak memory: 40093.42 MiB, increment: 31646.50 MiB
Wall time: 30.9 s

[eroell]

Indeed, ~3-7x faster for me & of course quite a bit more memory efficient (quickly checked with scalene).

Tests keep working (for seurat_v3/seurat_v3_paper somewhat tight numeric comparison with Seurat results), nice.

[ivirshup]

Looks good. Kinda suprised how much faster this is.

You could probably get another big speed up by operating on the whole matrix at once, and not creating batch specific matrices.

But I think fine as is.

I added a couple points to address, but that's just naming and docs.

[ivirshup]

Would also be nice to have a timing benchmark for this.

[Intron7]

It is the whole matrix for each batch. It's just called batch_counts because I needed to make sure the format is csr

[ivirshup]

I was thinking more of getting rid of the whole for b in np.unique(batch_info) loop and doing the whole thing in two passes over the matrix.

But LGTM, and I think ready to merge once those tests get fixed

[Intron7]

I don't know what makes the tests fail with dask in utils