Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix neighbors transformer shortcut #3444

Closed
wants to merge 7 commits into from
Closed

Conversation

flying-sheep
Copy link
Member

@flying-sheep flying-sheep commented Jan 20, 2025

  • Release notes not necessary because:

I don’t get it, what does numpy mean here?

Not equal to tolerance rtol=1e-05, atol=0

Mismatched elements: 6 / 10000 (0.06%)
Max absolute difference: 5.96046448e-08
Max relative difference: 5.51590204e-14
 x: array([[0.      , 0.      , 0.      , ..., 0.      , 0.      , 0.      ],
       [0.      , 0.      , 0.      , ..., 0.      , 0.      , 0.      ],
       [0.      , 0.      , 0.      , ..., 3.39297 , 0.      , 0.      ],...
 y: array([[0.      , 0.      , 0.      , ..., 0.      , 0.      , 0.      ],
       [0.      , 0.      , 0.      , ..., 0.      , 0.      , 0.      ],
       [0.      , 0.      , 0.      , ..., 3.39297 , 0.      , 0.      ],...

Copy link

codecov bot commented Jan 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.50%. Comparing base (75c246d) to head (51a9e22).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3444      +/-   ##
==========================================
+ Coverage   75.45%   75.50%   +0.05%     
==========================================
  Files         113      114       +1     
  Lines       13243    13282      +39     
==========================================
+ Hits         9992    10029      +37     
- Misses       3251     3253       +2     
Files with missing lines Coverage Δ
src/scanpy/neighbors/__init__.py 80.31% <100.00%> (+0.10%) ⬆️
src/scanpy/neighbors/_backends/pairwise.py 100.00% <100.00%> (ø)
src/scanpy/neighbors/_types.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

Copy link

scverse-benchmark bot commented Jan 21, 2025

Benchmark changes

Change Before [75c246d] After [51a9e22] Ratio Benchmark (Parameter)
+ 18.6±0.5ms 24.4±0.9ms 1.31 preprocessing_counts.FastSuite.time_calculate_qc_metrics('bmmc', 'counts')
- 1.04±0.03ms 698±30μs 0.67 preprocessing_log.FastSuite.time_mean_var('pbmc3k', 'off-axis')

Comparison: https://github.com/scverse/scanpy/compare/75c246d191aaabb02e6cce9d926de9fb40dbc77e..51a9e22ae1f1d1f124858207777e55134c708962
Last changed: Thu, 23 Jan 2025 12:39:00 +0000

More details: https://github.com/scverse/scanpy/pull/3444/checks?check_run_id=36056403402

@flying-sheep
Copy link
Member Author

Interesting, seems like pairwise is exactly as fast, but uses a lot more memory.

Since we only use it in the shortcut for small datasets, that’s not that big a deal, but one point against going back to pairwise

@flying-sheep flying-sheep added this to the 1.11.0 milestone Jan 21, 2025
@keller-mark keller-mark mentioned this pull request Jan 21, 2025
@flying-sheep
Copy link
Member Author

this doesn‘t do anything when there is no duplicated data, so since we don’t support duplicated data, we don’t want this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Leiden clustering returns different results for Scanpy 1.10.4 and 1.9.3
1 participant