You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "petern48 (via GitHub)" <gi...@apache.org> on 2025/09/02 15:58:09 UTC
[PR] WIP: Pytest benchmark proposal [sedona-db]
petern48 opened a new pull request, #10:
URL: https://github.com/apache/sedona-db/pull/10
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Pytest benchmark for comparing against other engines locally [sedona-db]
Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu commented on PR #10:
URL: https://github.com/apache/sedona-db/pull/10#issuecomment-3258460139
Does this mean all the performance numbers we saw yesterday are wrong?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Pytest benchmark proposal [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #10:
URL: https://github.com/apache/sedona-db/pull/10#discussion_r2316550987
##########
python/sedonadb/benchmarks/test_bench_base.py:
##########
@@ -0,0 +1,25 @@
+from sedonadb.testing import DuckDB, PostGIS, SedonaDB
+
+
+class TestBenchBase:
+ def setup_class(self):
+ self.sedonadb = SedonaDB.create_or_skip()
+ self.postgis = PostGIS.create_or_skip()
+ self.duckdb = DuckDB.create_or_skip()
+
+ # Setup tables
+ num_rows = 10000
+ create_points_query = f"CREATE TABLE points AS SELECT ST_GeomFromText('POINT(0 0)') AS geom FROM range({num_rows})"
Review Comment:
The `DBEngine` subclass has this abstracted already such that you can create a table from a GeoParquet file or GeoPandas data frame. You can use the `geoarrow_data` fixture to write benchmarks against actual data, or you can use the `sd_random_geometry()` table function to generate it (Kristin's join integration tests are a great example).
Probably synthetic data makes sense here: points, segments (linestrings with a vertex count of 2), polygon, complex_linestring, complex_polygon. The number of batches could be configurable so that you can run tiny benchmarks or big benchmarks (this is what we do in Rust, too).
##########
python/sedonadb/benchmarks/test_functions.py:
##########
@@ -0,0 +1,115 @@
+import pytest
+from test_bench_base import TestBenchBase
+from sedonadb.testing import DuckDB, geom_or_null, PostGIS, SedonaDB, val_or_null
+
+
+class TestBenchFunctions(TestBenchBase):
+ @pytest.mark.parametrize("eng", [SedonaDB, PostGIS, DuckDB])
+ def test_st_area(self, benchmark, eng):
+ eng = self._get_eng(eng)
+
+ def queries():
+ for geom in [
+ "POINT EMPTY",
+ "POINT(1 1)",
+ "LINESTRING(0 0, 1 1)",
+ "POLYGON((0 0, 1 0, 1 1, 0 1, 0 0))",
+ "MULTIPOLYGON(((0 0, 1 0, 1 1, 0 1, 0 0), (0.5 0.5, 0.6 0.6, 0.5 0.7, 0.4 0.6, 0.5 0.5)))",
+ "GEOMETRYCOLLECTION(POINT(0 0), LINESTRING(0 0, 1 1), POLYGON((0 0, 1 0, 1 1, 0 1, 0 0)))",
+ ]:
+ eng.execute_and_collect(f"SELECT ST_Area({geom_or_null(geom)})")
Review Comment:
Rather than unit tests, where we want to parameterize every single case, here we probably want to target specific queries. For example, for ST_Area(), we'd like to benchmark performance with against a simple polygon and a complex polygon and we're not so interested in the performance of the corner cases.
##########
python/sedonadb/benchmarks/test_bench_base.py:
##########
Review Comment:
I think this should live in just `benchmarks/`, because running benchmarks is generally time-consuming and done less frequently than unit tests. This will require some repeating of the fixtures but you can still use the `sedonadb.testing` module.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Pytest benchmark proposal [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #10:
URL: https://github.com/apache/sedona-db/pull/10#discussion_r2316939771
##########
python/sedonadb/benchmarks/test_bench_base.py:
##########
Review Comment:
Yes! (i.e., so they don't run by default or show a confusing number of skips when you type `pytest`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Pytest benchmark for comparing against other engines locally [sedona-db]
Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu merged PR #10:
URL: https://github.com/apache/sedona-db/pull/10
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Pytest benchmark for comparing against other engines locally [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #10:
URL: https://github.com/apache/sedona-db/pull/10#discussion_r2317683940
##########
benchmarks/test_bench_base.py:
##########
@@ -0,0 +1,78 @@
+import json
+from sedonadb.testing import DuckDB, PostGIS, SedonaDB
Review Comment:
These files all need the license header (I don't have a CI check set up for this yet 😬 )
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Pytest benchmark for comparing against other engines locally [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on PR #10:
URL: https://github.com/apache/sedona-db/pull/10#issuecomment-3258539697
> Does this mean all the performance numbers we saw yesterday are wrong?
I don't debate the diagnostics here, but I would be surprised if DuckDB's Python package was configured to use one thread by default always, and that this wasn't caught for the entire lifecycle of the 1.3 release. There are a number of things we need to consider on top of yesterday's numbers including this!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Pytest benchmark proposal [sedona-db]
Posted by "petern48 (via GitHub)" <gi...@apache.org>.
petern48 commented on code in PR #10:
URL: https://github.com/apache/sedona-db/pull/10#discussion_r2317230428
##########
python/sedonadb/benchmarks/test_bench_base.py:
##########
@@ -0,0 +1,25 @@
+from sedonadb.testing import DuckDB, PostGIS, SedonaDB
+
+
+class TestBenchBase:
+ def setup_class(self):
+ self.sedonadb = SedonaDB.create_or_skip()
+ self.postgis = PostGIS.create_or_skip()
+ self.duckdb = DuckDB.create_or_skip()
+
+ # Setup tables
+ num_rows = 10000
+ create_points_query = f"CREATE TABLE points AS SELECT ST_GeomFromText('POINT(0 0)') AS geom FROM range({num_rows})"
Review Comment:
How exactly would you want to quanity complex vs non-complex?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Pytest benchmark proposal [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on PR #10:
URL: https://github.com/apache/sedona-db/pull/10#issuecomment-3246937605
Awesome!
> How much do we care to drill down?
Not that far! The tests are all about correctness and corner cases...here we can stick to the most common cases. Most functions shouldn't need more than one or two benchmarks (one on a simple geometry, which is a benchmark of our per-geometry overhead, and one for complex geometry, which is more a test of the underlying implementation).
> I'm not sure how 1. we should make this configurable or 2. to what extent we should make it configurable.
No need to make it configurable now, but maybe rename the tables to `_small` and `_large` in case we decide to change those numbers?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Pytest benchmark proposal [sedona-db]
Posted by "petern48 (via GitHub)" <gi...@apache.org>.
petern48 commented on code in PR #10:
URL: https://github.com/apache/sedona-db/pull/10#discussion_r2316925067
##########
python/sedonadb/benchmarks/test_bench_base.py:
##########
Review Comment:
You mean move this from `python/sedonadb/benchmarks/` to `benchmarks/` outside of the python folder?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Pytest benchmark for comparing against other engines locally [sedona-db]
Posted by "petern48 (via GitHub)" <gi...@apache.org>.
petern48 commented on PR #10:
URL: https://github.com/apache/sedona-db/pull/10#issuecomment-3247043686
I added the separate 'simple' and 'complex' tables and removed the small sizing (using the original large size for everything). I didn't see much value from that dimension at the moment and this mimics the rust tests the most. I also don't think it makes sense to integrate with CI for now, since there might still be options other than`pytest-benchmark` that are better suited for our needs. Just wanted a nice quick tool for comparing against different engines locally.
Here's an example of how it can be used locally at the moment. The below command will group the results of simple and complex separately.
`pytest --benchmark-group-by=func,param:table test_functions.py::TestBenchFunctions::test_st_area`
<img width="1504" height="212" alt="image" src="https://github.com/user-attachments/assets/008aeb01-014a-4b45-8277-ff67d3d0bc97" />
WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Pytest benchmark proposal [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #10:
URL: https://github.com/apache/sedona-db/pull/10#discussion_r2317263257
##########
python/sedonadb/benchmarks/test_bench_base.py:
##########
@@ -0,0 +1,25 @@
+from sedonadb.testing import DuckDB, PostGIS, SedonaDB
+
+
+class TestBenchBase:
+ def setup_class(self):
+ self.sedonadb = SedonaDB.create_or_skip()
+ self.postgis = PostGIS.create_or_skip()
+ self.duckdb = DuckDB.create_or_skip()
+
+ # Setup tables
+ num_rows = 10000
+ create_points_query = f"CREATE TABLE points AS SELECT ST_GeomFromText('POINT(0 0)') AS geom FROM range({num_rows})"
Review Comment:
It looks like you found `vertices_per_linestring_range`. I use the numbers 10 ("simple") and 500 ("complex") in the Rust benchmarks, which is sort of arbitrary but did the trick of weeding out predicate implementations that weren't using a prepared geometry (particularly when one side was a scalar). Totally optional!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Pytest benchmark proposal [sedona-db]
Posted by "petern48 (via GitHub)" <gi...@apache.org>.
petern48 commented on PR #10:
URL: https://github.com/apache/sedona-db/pull/10#issuecomment-3245933598
Just pushing this somewhere for now. There are other benchmark library options too, so we don't need to commit to this one. I just found this very easy to setup and use.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Pytest benchmark for comparing against other engines locally [sedona-db]
Posted by "Kontinuation (via GitHub)" <gi...@apache.org>.
Kontinuation commented on PR #10:
URL: https://github.com/apache/sedona-db/pull/10#issuecomment-3257793272
I ran the benchmark and found that sedona-db uses all the CPU cores to run the benchmarking query, while DuckDB only uses one single core (on CPython main thread). This makes the benchmark results of sedona-db and DuckDB not directly comparable.
Have you ran into this issue before? Should we configure the benchmarked engines to force using single thread to execute the queries?
```
```
<img width="3428" height="964" alt="image" src="https://github.com/user-attachments/assets/e3c76985-3130-4535-a208-16479f4b98ec" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Pytest benchmark proposal [sedona-db]
Posted by "petern48 (via GitHub)" <gi...@apache.org>.
petern48 commented on PR #10:
URL: https://github.com/apache/sedona-db/pull/10#issuecomment-3246881588
I've changed it so that we generate columns of random geometries `points_10_000`, `polygons_10_000`, `polygons_100_000`, etc. I'm not sure how 1. we should make this configurable or 2. to what extent we should make it configurable.
Like if we do different geometry types, simple / complex, and number of geometries, I feel that's a lot of dimensions. How much do we care to drill down?
Looking at the current implementation of `test_st_area` (which is parametrized, unlike the rest). We can group by table (dataset size, etc) and compare the engines at a more granular level.
(notice duckdb wins for one of the simpler datasets here, although sedonadb is faster for the rest and overall)
`pytest --benchmark-group-by=param:table test_functions.py::TestBenchFunctions::test_st_area`
<img width="1392" height="400" alt="image" src="https://github.com/user-attachments/assets/b1cd51a2-e889-4258-ba3f-576f53ce5ee2" />
or we can can just benchmark them at the function level (e.g st_buffer)
`pytest --benchmark-group-by=func test_functions.py::TestBenchFunctions::test_st_buffer`
<img width="1260" height="103" alt="image" src="https://github.com/user-attachments/assets/ee825604-fc62-4ebe-987e-a2ed7e8ed1bc" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Pytest benchmark for comparing against other engines locally [sedona-db]
Posted by "petern48 (via GitHub)" <gi...@apache.org>.
petern48 commented on PR #10:
URL: https://github.com/apache/sedona-db/pull/10#issuecomment-3258876565
very good catch 😬
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org