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 •
edited
Loading

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 •
edited
Loading

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
@flying-sheep
flying-sheep requested a review from ilan-gold May 8, 2025 12:14
ilan-gold
ilan-gold approved these changes May 8, 2025
View reviewed changes

@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 Resolved
@scverse-benchmark

scverse-benchmark Bot commented May 8, 2025 •
edited
Loading

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 •
edited
Loading

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
@flying-sheep
Backport PR #3598: Switch to fast-array-utils
b0e884e
@flying-sheep flying-sheep mentioned this pull request May 12, 2025
Closed
@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
@flying-sheep @rgoya @selmanozleyen
Switch to fast-array-utils (#3598)
be7c0cd 
Co-authored-by: Rodrigo Goya <[email protected]>
ilan-gold added a commit that referenced this pull request May 19, 2025
@Intron7 @flying-sheep
@selmanozleyen @pre-commit-ci @swstkm @rgoya @ch1ru @ilan-gold
normalize_total with numba (#3571)
55bfe34 
* update normalize_total & remove dep

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

Co-authored-by: Philipp A. <[email protected]>

* 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. <[email protected]>

* Fix typo (#3555)

Co-authored-by: Phil Schaf <[email protected]>

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

Co-authored-by: Rodrigo Goya <[email protected]>

* tSNE components parameter (#2803)

Co-authored-by: Phil Schaf <[email protected]>

* Simplify scale (#3351)

Co-authored-by: Intron7 <[email protected]>
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. <[email protected]>
Co-authored-by: Selman Özleyen <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Swastik Mishra <[email protected]>
Co-authored-by: Rodrigo Goya <[email protected]>
Co-authored-by: Kitsune <[email protected]>
Co-authored-by: selmanozleyen <[email protected]>
Co-authored-by: ilan-gold <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ilan-gold ilan-gold ilan-gold approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.12.0

Development

Successfully merging this pull request may close these issues.

Switch to fast-array-utils

3 participants

@flying-sheep @ilan-gold @rgoya