Fix point bounding box query bug (occurring for non-trivial transformations within non-matching 3D axes) by LucaMarconato · Pull Request #1187 · scverse/spatialdata

LucaMarconato · 2026-08-20T13:24:38Z

codecov · 2026-08-20T13:58:45Z

Codecov Report

❌ Patch coverage is 94.69027% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.85%. Comparing base (7944486) to head (3b05663).

Files with missing lines	Patch %	Lines
src/spatialdata/_core/query/spatial_query.py	89.13%	5 Missing ⚠️
src/spatialdata/transformations/transformations.py	97.95%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #1187 +/- ##
==========================================
- Coverage 92.40% 91.85% -0.56% 
==========================================
Files 52 53 +1 Lines 7872 7928 +56 ==========================================
+ Hits 7274 7282 +8 - Misses 598 646 +48

Files with missing lines	Coverage Δ
src/spatialdata/_core/operations/rasterize.py	`90.60% <100.00%> (ø)`
src/spatialdata/_core/query/_utils.py	`93.26% <100.00%> (ø)`
src/spatialdata/_types.py	`100.00% <100.00%> (ø)`
src/spatialdata/_utils.py	`85.43% <100.00%> (-0.10%)`	⬇️
src/spatialdata/utils/points.py	`100.00% <100.00%> (ø)`
src/spatialdata/transformations/transformations.py	`84.12% <97.95%> (-6.68%)`	⬇️
src/spatialdata/_core/query/spatial_query.py	`94.72% <89.13%> (-0.81%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

LucaMarconato · 2026-08-20T13:59:24Z

For reviewing:

the main idea is to revert the code that was available before https://vscode.dev/github/scverse/spatialdata/pull/1104/changes, while still maintaining the strategy of that PR: return early when the transformation is simple, to avoid having to transform the data. Practically: for reviewing it's useful to diff against the original code before Speedup for bounding_box_query #1104
instead of manually checking if a transformation is identity or scale, we now use _decompose_transformation_full(). If the transformation has no rotation and no shear, we are good to go: the query in the intrinsic space using the backward-transformed bounding box is equivalent to querying in the target space. So we return early (save computations).
I kept the plots in the tests; before merging let's remove the commented code.

LucaMarconato · 2026-08-25T12:35:06Z

(@ajkswamy did a rebase to meet the requirements of the stacked PR machinery since a new PR got merged into main while this was open).

ported from transfo: supporting z and c; improved order of returned transformations also: split into simple/full; changed return type to tuples; supporting permutation of input/output axes for the transformation

…point bb queries

…marks

ajkswamy · 2026-08-26T11:00:15Z

@LucaMarconato I've added some commits to the PR, some before and some after your commits (1d775ad and 89ca4cd). The commits before were needed for fixing the benchmarking setup and adding a new benchmark. This helps us compare the benchmark results before and after your commits. The commits after were minor refactors/cleanups, feel free to revert them if you see fit.

Your changes for fixing the issue look good to me, in the context of the current issue. We could do other improvements like better typing and code refactoring for readability and maintainability, but we can do that later as needed.

I ran benchmarks on my Windows Laptop (i7-11800H, 8 cores, 2.3 Ghz, 16GB RAM) to compare the execution times of the newly added benchmark before and after your changes. The results showed a significant increase of about 35%. It would be good to add another benchmark with larger points data to help get a sense of how the execution time might scale with larger data.

Here are the benchmarking commands and results:

Comparing benchmarks of `bbf9c0` with `89ca4c` (i.e., just your commits)

PS .....> asv continuous --show-stderr -a rounds=10 -b TimeQueriesWithScaleTransformations bbf9c0ae5ae4adc4e25198e9704f454b79b224a3 89ca4cd06c3ff6c7755aee2a013941f3cdfa7a58
· Creating environments
· Discovering benchmarks..
·· Uninstalling from virtualenv-py3.12.
·· Building 89ca4cd0 <fix/bounding-box-query-axes-dependent-results~6> for virtualenv-py3.12
·· Installing 89ca4cd0 <fix/bounding-box-query-axes-dependent-results~6> into virtualenv-py3.12....
· Running 2 total benchmarks (2 commits * 1 environments * 1 benchmarks)
[ 0.00%] · For spatialdata commit bbf9c0ae <add-benchmark> (round 1/10):
[ 0.00%] ·· Building for virtualenv-py3.12......
[ 0.00%] ·· Benchmarking virtualenv-py3.12
[ 5.00%] ··· Running (spatialdata_benchmark.TimeQueriesWithScaleTransformations.time_bbquery_scale_transform--).
[ 5.00%] · For spatialdata commit 89ca4cd0 <fix/bounding-box-query-axes-dependent-results~6> (round 1/10):
[ 5.00%] ·· Building for virtualenv-py3.12.......
[ 5.00%] ·· Benchmarking virtualenv-py3.12
[10.00%] ··· Running (spatialdata_benchmark.TimeQueriesWithScaleTransformations.time_bbquery_scale_transform--).
[10.00%] · For spatialdata commit 89ca4cd0 <fix/bounding-box-query-axes-dependent-results~6> (round 2/10):
[10.00%] ·· Benchmarking virtualenv-py3.12
[15.00%] ··· Running (spatialdata_benchmark.TimeQueriesWithScaleTransformations.time_bbquery_scale_transform--).
[15.00%] · For spatialdata commit bbf9c0ae <add-benchmark> (round 2/10):
[15.00%] ·· Building for virtualenv-py3.12......
...
...
[90.00%] · For spatialdata commit 89ca4cd0 <fix/bounding-box-query-axes-dependent-results~6> (round 10/10):
[90.00%] ·· Benchmarking virtualenv-py3.12
[95.00%] ··· spatialdata_benchmark.TimeQueriesWithScaleTransformations.time_bbquery_scale_transform 9.47±1ms
[95.00%] · For spatialdata commit bbf9c0ae <add-benchmark> (round 10/10):
[95.00%] ·· Building for virtualenv-py3.12......
[95.00%] ·· Benchmarking virtualenv-py3.12
[100.00%] ··· spatialdata_benchmark.TimeQueriesWithScaleTransformations.time_bbquery_scale_transform 6.74±1ms
| Change | Before [bbf9c0ae] <add-benchmark> | After [89ca4cd0] <fix/bounding-box-query-axes-dependent-results~6> | Ratio | Benchmark (Parameter) |
|----------|-------------------------------------|----------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------|
| + | 6.74±1ms | 9.47±1ms | 1.41 | spatialdata_benchmark.TimeQueriesWithScaleTransformations.time_bbquery_scale_transform |
SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE DECREASED.

Comparing benchmarks of `bbf9c0` with PR `HEAD` (i.e. including my commits after yours)

PS ......> asv continuous --show-stderr -a rounds=10 -b TimeQueriesWithScaleTransformations bbf9c0ae5ae4adc4e25198e9704f454b79b224a3 HEAD
· Creating environments
· Discovering benchmarks..
·· Uninstalling from virtualenv-py3.12.
·· Building 38e16f1a <fix/bounding-box-query-axes-dependent-results> for virtualenv-py3.12
·· Installing 38e16f1a <fix/bounding-box-query-axes-dependent-results> into virtualenv-py3.12....
· Running 2 total benchmarks (2 commits * 1 environments * 1 benchmarks)
[ 0.00%] · For spatialdata commit bbf9c0ae <add-benchmark> (round 1/10):
[ 0.00%] ·· Building for virtualenv-py3.12.......
[ 0.00%] ·· Benchmarking virtualenv-py3.12
[ 5.00%] ··· Running (spatialdata_benchmark.TimeQueriesWithScaleTransformations.time_bbquery_scale_transform--).
[ 5.00%] · For spatialdata commit 38e16f1a <fix/bounding-box-query-axes-dependent-results> (round 1/10):
[ 5.00%] ·· Building for virtualenv-py3.12.......
[ 5.00%] ·· Benchmarking virtualenv-py3.12
[10.00%] ··· Running (spatialdata_benchmark.TimeQueriesWithScaleTransformations.time_bbquery_scale_transform--).
[10.00%] · For spatialdata commit 38e16f1a <fix/bounding-box-query-axes-dependent-results> (round 2/10):
[10.00%] ·· Benchmarking virtualenv-py3.12
[15.00%] ··· Running (spatialdata_benchmark.TimeQueriesWithScaleTransformations.time_bbquery_scale_transform--).
[15.00%] · For spatialdata commit bbf9c0ae <add-benchmark> (round 2/10):
[15.00%] ·· Building for virtualenv-py3.12.......
...
...
[90.00%] · For spatialdata commit 38e16f1a <fix/bounding-box-query-axes-dependent-results> (round 10/10):
[90.00%] ·· Benchmarking virtualenv-py3.12
[95.00%] ··· spatialdata_benchmark.TimeQueriesWithScaleTransformations.time_bbquery_scale_transform 8.94±0.5ms
[95.00%] · For spatialdata commit bbf9c0ae <add-benchmark> (round 10/10):
[95.00%] ·· Building for virtualenv-py3.12.......
[95.00%] ·· Benchmarking virtualenv-py3.12
[100.00%] ··· spatialdata_benchmark.TimeQueriesWithScaleTransformations.time_bbquery_scale_transform 6.66±1ms
| Change | Before [bbf9c0ae] <add-benchmark> | After [38e16f1a] <fix/bounding-box-query-axes-dependent-results> | Ratio | Benchmark (Parameter) |
|----------|-------------------------------------|--------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------|
| + | 6.66±1ms | 8.94±0.5ms | 1.34 | spatialdata_benchmark.TimeQueriesWithScaleTransformations.time_bbquery_scale_transform |
SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE DECREASED.

ajkswamy

LGTM, I've added some commits, please see my last comment on the PR

LucaMarconato · 2026-08-27T17:04:20Z

Thanks for the review for extending the asv benchmarks.

I have ran new benchmarks with larger data sizes and the regression in performance is only occurring for small-medium sample sizes. With large data (most interesting use case) the performance is not significantly different than before this PR.

If in the future we observe the need to improve performance also for small sample sizes, we can open a follow up PR centered on performance.

Ready to merge! 🚀


Results — mean ± sample std (ms)
┌─────────────┬──────────────────┬───────────────────────┬─────────┬─────────┐
│ points │ before bbf9c0ae │ after / HEAD 7ae304fc │ ratio R │ ± (SEM) │
├─────────────┼──────────────────┼───────────────────────┼─────────┼─────────┤
│ 10 │ 2.65 ± 0.45 │ 3.71 ± 0.54 │ 1.40 │ 0.08 │
├─────────────┼──────────────────┼───────────────────────┼─────────┼─────────┤
│ 100 │ 2.11 ± 0.16 │ 6.67 ± 10.96 ⚠︎ │ 3.16 │ 1.34 │
├─────────────┼──────────────────┼───────────────────────┼─────────┼─────────┤
│ 1,000 │ 2.05 ± 0.11 │ 3.11 ± 0.17 │ 1.52 │ 0.03 │
├─────────────┼──────────────────┼───────────────────────┼─────────┼─────────┤
│ 10,000 │ 2.39 ± 0.32 │ 3.39 ± 0.27 │ 1.42 │ 0.06 │
├─────────────┼──────────────────┼───────────────────────┼─────────┼─────────┤
│ 100,000 │ 3.85 ± 0.31 │ 5.64 ± 1.33 │ 1.47 │ 0.09 │
├─────────────┼──────────────────┼───────────────────────┼─────────┼─────────┤
│ 1,000,000 │ 24.76 ± 11.64 │ 23.29 ± 6.74 │ 0.94 │ 0.13 │
├─────────────┼──────────────────┼───────────────────────┼─────────┼─────────┤
│ 10,000,000 │ 167.5 ± 46.6 │ 172.8 ± 47.0 │ 1.03 │ 0.10 │
├─────────────┼──────────────────┼───────────────────────┼─────────┼─────────┤
│ 100,000,000 │ 2731 ± 291 (n=3) │ 2715 ± 327 (n=3) │ 0.99 │ 0.09 │
└─────────────┴──────────────────┴───────────────────────┴─────────┴─────────┘

LucaMarconato changed the base branch from main to improve/affine-decompositionAugust 20, 2026 13:27

LucaMarconato force-pushed the fix/bounding-box-query-axes-dependent-results branch from f8c57d5 to 1600425Compare August 20, 2026 13:28

LucaMarconato marked this pull request as ready for review August 20, 2026 13:59

ajkswamy force-pushed the fix/bounding-box-query-axes-dependent-results branch from 7021386 to 98c7d85Compare August 21, 2026 09:38

LucaMarconato commented Aug 21, 2026
View reviewed changes

Comment threadtests/core/query/test_spatial_query.py Outdated

LucaMarconato commented Aug 21, 2026
View reviewed changes

Comment threadtests/core/query/test_spatial_query.py Outdated

LucaMarconato force-pushed the fix/bounding-box-query-axes-dependent-results branch from 98c7d85 to c203796Compare August 25, 2026 12:30

Base automatically changed from improve/affine-decomposition to mainAugust 25, 2026 12:33

LucaMarconato force-pushed the fix/bounding-box-query-axes-dependent-results branch from c203796 to a8c1d7aCompare August 25, 2026 12:33

LucaMarconatoand others added 15 commits August 25, 2026 17:07

refactor: improve affine decomposition
a09b9cf
ported from transfo: supporting z and c; improved order of returned transformations also: split into simple/full; changed return type to tuples; supporting permutation of input/output axes for the transformation

fix: benchmarks/README.md
caa1901

refac: move _make_points to new utils file
150d4f7

feat: new benchmark for quering points with scale transform
c295094

fix: importing utils in spatial_benchmark.py
3093327

fix(asv.conf.json): installing torch
bbf9c0a

wip refactoring of bounding box point queries
1d775ad

fix: use the new _decompose_transformation_full() to return early in …
89ca4cd
…point bb queries

refac: type def Number
26b42b6

refac: using custom types
79ee707

fix: cleanup comments
aff8f99

fix: cleanup unused function
8a9e8c0

fix: cleanup leftover comment
e87ee0f

fix: minor test improvement
38e16f1

feat(benchmarks/README): added info about running <x> rounds of bench…
7f89b13
…marks

fix(test_transformations.py): removed unused unresolvable import
f4cfb43

ajkswamy force-pushed the fix/bounding-box-query-axes-dependent-results branch from a8c1d7a to f4cfb43Compare August 26, 2026 10:17

ajkswamy self-requested a review August 26, 2026 11:00

ajkswamy approved these changes Aug 26, 2026
View reviewed changes

Comment threadsrc/spatialdata/_core/query/spatial_query.py Outdated
Comment threadsrc/spatialdata/_core/query/spatial_query.py

LucaMarconato added 4 commits August 27, 2026 15:34

fix: bigger query benchmark dataset; moved comment
252a8d6

fix
688791a

fix: better comments for bounding box queries; benchmarking scaling
7ae304f

bench: up to 100M points for bounding box queries
3b05663

LucaMarconato merged commit c12548f into mainAug 27, 2026
10 checks passed

LucaMarconato deleted the fix/bounding-box-query-axes-dependent-results branch August 27, 2026 17:08

Navigation Menu

Uh oh!

Fix point bounding box query bug (occurring for non-trivial transformations within non-matching 3D axes) - #1187

Fix point bounding box query bug (occurring for non-trivial transformations within non-matching 3D axes)#1187
LucaMarconato merged 20 commits into
mainfrom
fix/bounding-box-query-axes-dependent-results

LucaMarconato commented Aug 20, 2026

Uh oh!

codecovBot commented Aug 20, 2026•
edited
Loading

Uh oh!

LucaMarconato commented Aug 20, 2026•
edited
Loading

Uh oh!

Uh oh!

Uh oh!

LucaMarconato commented Aug 25, 2026

Uh oh!

ajkswamy commented Aug 26, 2026

Uh oh!

ajkswamy left a comment

Uh oh!

Uh oh!

Uh oh!

LucaMarconato commented Aug 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

LucaMarconato commented Aug 20, 2026

Uh oh!

codecovBot commented Aug 20, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

LucaMarconato commented Aug 20, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LucaMarconato commented Aug 25, 2026

Uh oh!

ajkswamy commented Aug 26, 2026

Uh oh!

ajkswamy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

LucaMarconato commented Aug 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecovBot commented Aug 20, 2026•
edited
Loading

LucaMarconato commented Aug 20, 2026•
edited
Loading