Skip to content

Switch to fast-array-utils - #3598

Merged
flying-sheep merged 18 commits into
mainfrom
pa/fast-array-utils
May 12, 2025
Merged

Switch to fast-array-utils#3598
flying-sheep merged 18 commits into
mainfrom
pa/fast-array-utils

Conversation

@flying-sheep

@flying-sheep flying-sheep commented Apr 17, 2025

Copy link
Copy Markdown
Member
  • Release notes not necessary because:

@flying-sheep flying-sheep added this to the 1.11.2 milestone Apr 28, 2025
@codecov

codecov Bot commented Apr 28, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.47368% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.68%. Comparing base (48fcaa9) to head (0a407af).
⚠️ Report is 48 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/scanpy/preprocessing/_normalization.py 85.71% 2 Missing ⚠️
src/scanpy/preprocessing/_qc.py 84.61% 2 Missing ⚠️
src/scanpy/tools/_leiden.py 50.00% 2 Missing ⚠️
src/scanpy/external/exporting.py 50.00% 1 Missing ⚠️
src/scanpy/preprocessing/_scrublet/sparse_utils.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3598      +/-   ##
==========================================
+ Coverage   75.67%   76.68%   +1.01%     
==========================================
  Files         114      113       -1     
  Lines       13143    12735     -408     
==========================================
- Hits         9946     9766     -180     
+ Misses       3197     2969     -228     
Files with missing lines Coverage Δ
src/scanpy/_utils/__init__.py 75.28% <100.00%> (-1.52%) ⬇️
...c/scanpy/experimental/pp/_highly_variable_genes.py 94.17% <100.00%> (+31.44%) ⬆️
src/scanpy/metrics/_common.py 91.13% <100.00%> (ø)
src/scanpy/metrics/_gearys_c.py 100.00% <100.00%> (+43.75%) ⬆️
src/scanpy/metrics/_morans_i.py 100.00% <100.00%> (+32.00%) ⬆️
src/scanpy/neighbors/_common.py 64.91% <100.00%> (ø)
...preprocessing/_deprecated/highly_variable_genes.py 93.40% <100.00%> (ø)
src/scanpy/preprocessing/_highly_variable_genes.py 93.70% <100.00%> (ø)
src/scanpy/preprocessing/_pca/_compat.py 100.00% <100.00%> (ø)
src/scanpy/preprocessing/_pca/_dask.py 91.46% <100.00%> (-0.11%) ⬇️
... and 10 more

@flying-sheep
flying-sheep marked this pull request as ready for review May 8, 2025 10:42

@ilan-gold ilan-gold left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable. What is to come of testing i.e., using fast-array-utils's mechanisms for that? We will do this in anndata first?

Comment thread src/scanpy/preprocessing/_normalization.py Outdated
@scverse-benchmark

scverse-benchmark Bot commented May 8, 2025

Copy link
Copy Markdown

Benchmark changes

Change Before [48fcaa9] After [0a407af] Ratio Benchmark (Parameter)
- 438M 396M 0.9 preprocessing_counts.FastSuite.peakmem_calculate_qc_metrics('pbmc3k', 'counts')
- 444M 398M 0.9 preprocessing_counts.FastSuite.peakmem_calculate_qc_metrics('pbmc3k', 'counts-off-axis')
- 443M 396M 0.89 preprocessing_counts.FastSuite.peakmem_log1p('pbmc3k', 'counts')
- 438M 398M 0.91 preprocessing_counts.FastSuite.peakmem_log1p('pbmc3k', 'counts-off-axis')
- 450M 404M 0.9 preprocessing_counts.FastSuite.peakmem_normalize_total('pbmc3k', 'counts')
- 453M 407M 0.9 preprocessing_counts.FastSuite.peakmem_normalize_total('pbmc3k', 'counts-off-axis')
- 20.7±0.7ms 12.0±0.1ms 0.58 preprocessing_counts.FastSuite.time_calculate_qc_metrics('bmmc', 'counts')
- 14.4±0.7ms 11.8±0.07ms 0.82 preprocessing_counts.FastSuite.time_calculate_qc_metrics('bmmc', 'counts-off-axis')
+ 1.74±0s 2.07±0.01s 1.19 preprocessing_counts.FastSuite.time_calculate_qc_metrics('lung93k', 'counts')
+ 1.41±0s 1.64±0.02s 1.16 preprocessing_counts.FastSuite.time_calculate_qc_metrics('lung93k', 'counts-off-axis')
- 7.04±0.05ms 4.57±0.02ms 0.65 preprocessing_counts.FastSuite.time_calculate_qc_metrics('pbmc68k_reduced', 'counts')
- 5.60±0.05ms 4.58±0.02ms 0.82 preprocessing_counts.FastSuite.time_calculate_qc_metrics('pbmc68k_reduced', 'counts-off-axis')
- 1.71±0.2ms 1.51±0.01ms 0.88 preprocessing_counts.FastSuite.time_log1p('bmmc', 'counts')
- 9.96±0.6ms 7.42±0.2ms 0.75 preprocessing_counts.FastSuite.time_log1p('pbmc3k', 'counts-off-axis')
- 15.9±0.5ms 10.4±0.6ms 0.66 preprocessing_counts.FastSuite.time_normalize_total('pbmc3k', 'counts')
- 480M 433M 0.9 preprocessing_counts.peakmem_filter_cells('pbmc3k', 'counts')
- 477M 431M 0.9 preprocessing_counts.peakmem_filter_cells('pbmc3k', 'counts-off-axis')
- 478M 429M 0.9 preprocessing_counts.peakmem_filter_genes('pbmc3k', 'counts')
- 595M 535M 0.9 preprocessing_log.peakmem_pca('pbmc3k', None)
+ 23.3±0.6ms 43.4±2ms 1.86 preprocessing_log.time_highly_variable_genes('pbmc3k', None)

Comparison: https://github.com/scverse/scanpy/compare/48fcaa980063f2cc25fc7b3a9703dcf1926686b1..0a407afa3232eaee2916dad2d5163df656b87180
Last changed: Mon, 12 May 2025 12:05:01 +0000

More details: https://github.com/scverse/scanpy/pull/3598/checks?check_run_id=42051919522

@flying-sheep

flying-sheep commented May 9, 2025

Copy link
Copy Markdown
Member Author

What is to come of testing i.e., using fast-array-utils's mechanisms for that? We will do this in anndata first?

yeah, I think we should do it in anndata first!

Change Before [48fcaa9] After [265701c] Ratio Benchmark (Parameter)
+ 22.5±0.3ms 43.0±2ms 1.91 preprocessing_log.time_highly_variable_genes('pbmc3k', None)

this is the only slightly concerning benchmark, I think I’ll look into that before merging

/edit: can’t reproduce locally

@flying-sheep
flying-sheep merged commit e0fc56a into main May 12, 2025
@flying-sheep
flying-sheep deleted the pa/fast-array-utils branch May 12, 2025 12:18
flying-sheep added a commit that referenced this pull request May 12, 2025
@scverse scverse deleted a comment from lumberbot-app Bot May 12, 2025
@flying-sheep flying-sheep modified the milestones: 1.11.2, 1.12.0 May 12, 2025
selmanozleyen pushed a commit that referenced this pull request May 19, 2025
Co-authored-by: Rodrigo Goya <rgoya@users.noreply.github.com>
ilan-gold added a commit that referenced this pull request May 19, 2025
* update normalize_total & remove dep

* refactor: `normalize_total` with Numba (#3593)

Co-authored-by: Philipp A. <flying-sheep@web.de>

* fix doctest

* add release note

* Switch from profimp to tuna for parsing import profiles (#3620)

* [pre-commit.ci] pre-commit autoupdate (#3622)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Philipp A. <flying-sheep@web.de>

* Fix typo (#3555)

Co-authored-by: Phil Schaf <flying-sheep@web.de>

* Switch to fast-array-utils (#3598)

Co-authored-by: Rodrigo Goya <rgoya@users.noreply.github.com>

* tSNE components parameter (#2803)

Co-authored-by: Phil Schaf <flying-sheep@web.de>

* Simplify scale (#3351)

Co-authored-by: Intron7 <severin.dicks@icloud.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* remove layers test

* explicit return and fix the info message

* remove unnecessary ravel's

* precision fix

* (fix): maintain data type properly.

* (fix): remove artifact storage

* (refactor): use nice f-a-u typing

---------

Co-authored-by: Philipp A. <flying-sheep@web.de>
Co-authored-by: Selman Özleyen <32667648+selmanozleyen@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Swastik Mishra <4453983+swstkm@users.noreply.github.com>
Co-authored-by: Rodrigo Goya <rgoya@users.noreply.github.com>
Co-authored-by: Kitsune <48340051+ch1ru@users.noreply.github.com>
Co-authored-by: selmanozleyen <syozleyen@gmail.com>
Co-authored-by: ilan-gold <ilanbassgold@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Switch to fast-array-utils

3 participants