Skip to content

chore(rust): Update to geo 0.32.0 #566

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.

Sign up for GitHub

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

Draft
paleolimbot wants to merge 6 commits into
base: main
Choose a base branch
from
+113 −63
Draft

chore(rust): Update to geo 0.32.0 #566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2ef2e47
update so it compiles
paleolimbot Jan 30, 2026
e3fa1e6
comment out failing tests
paleolimbot Jan 30, 2026
eb377f8
tests passing
paleolimbot Jan 30, 2026
ab59d09
also update i_overlay
paleolimbot Jan 30, 2026
f7d27b2
Merge remote-tracking branch 'upstream/main' into geo-32
paleolimbot May 4, 2026
32aa8db
still failing but updated
paleolimbot May 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 41 additions & 4 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ env_logger = "0.11"
fastrand = "2.4"
float_next_after = "2"
futures = "0.3"
geo = "0.31.0"
geo = "0.33.1"
geo-index = { version = "0.3.4", features = ["use-geo_0_31"] }
geo-traits = "0.3.0"
geo-types = "0.7.17"
Expand Down
80 changes: 40 additions & 40 deletions rust/sedona-geo-generic-alg/src/algorithm/line_measures/metric_spaces/euclidean/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1571,28 +1571,28 @@ mod tests {
}
}

#[test]
fn test_random_linestring_to_linestring_distance() {
// Test linestring-to-linestring distance with random inputs
for i in 0..100 {
let seed1 = 77777 + i * 59;
let seed2 = 88888 + i * 61;

let ls1 = generate_random_linestring(seed1, 3 + (i % 3) as usize); // 3-5 points
let ls2 = generate_random_linestring(seed2, 3 + ((i + 1) % 3) as usize); // 3-5 points

let concrete_dist = Euclidean.distance(&ls1, &ls2);
// Use our actual generic implementation via nearest_neighbour_distance
let generic_dist = nearest_neighbour_distance(&ls1, &ls2);

assert_relative_eq!(
concrete_dist,
generic_dist,
epsilon = 1e-10,
max_relative = 1e-10
);
}
}
// #[test]
// fn test_random_linestring_to_linestring_distance() {
// // Test linestring-to-linestring distance with random inputs
// for i in 0..100 {
// let seed1 = 77777 + i * 59;
// let seed2 = 88888 + i * 61;

// let ls1 = generate_random_linestring(seed1, 3 + (i % 3) as usize); // 3-5 points
// let ls2 = generate_random_linestring(seed2, 3 + ((i + 1) % 3) as usize); // 3-5 points

// let concrete_dist = Euclidean.distance(&ls1, &ls2);
// // Use our actual generic implementation via nearest_neighbour_distance
// let generic_dist = nearest_neighbour_distance(&ls1, &ls2);

// assert_relative_eq!(
// concrete_dist,
// generic_dist,
// epsilon = 1e-10,
// max_relative = 1e-10
// );
// }
// }
Comment on lines +1574 to +1595
Copy link
Copy Markdown
Member Author

@paleolimbot paleolimbot Jan 30, 2026

Choose a reason for hiding this comment

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

@Kontinuation Can you look into these two tests? I am wondering if the mismatch is a bug on our side or a bug on their side (or unrelated and I'm not understanding the test).

The failure is:

thread 'algorithm::line_measures::metric_spaces::euclidean::utils::tests::test_random_linestring_to_linestring_distance' panicked at rust/sedona-geo-generic-alg/src/algorithm/line_measures/metric_spaces/euclidean/utils.rs:1588:13:
assert_relative_eq!(concrete_dist, generic_dist, epsilon = 1e-10, max_relative = 1e-10)

    left  = 28.34636916784935
    right = 21.713575661323034

(i.e., the algorithm here is giving smaller distances than whatever is producing concrete_dist)

Copy link
Copy Markdown
Member

@Kontinuation Kontinuation Feb 4, 2026

Choose a reason for hiding this comment

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

Preliminary investigations indicate that our calculations are correct, and geo 0.32.0 is incorrect.

I'll look into it further and see if I can submit a patch to georust/geo.

Copy link
Copy Markdown
Member

@Kontinuation Kontinuation Feb 4, 2026

Choose a reason for hiding this comment

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

I have submitted georust/geo#1499 to fix this.


#[test]
fn test_random_polygon_to_polygon_distance() {
Expand Down Expand Up @@ -1638,27 +1638,27 @@ mod tests {
}
}

#[test]
fn test_random_linestring_to_polygon_distance() {
// Test linestring-to-polygon distance with random inputs
for i in 0..100 {
let seed1 = 14141 + i * 83;
let seed2 = 15151 + i * 89;
// #[test]
// fn test_random_linestring_to_polygon_distance() {
// // Test linestring-to-polygon distance with random inputs
// for i in 0..100 {
// let seed1 = 14141 + i * 83;
// let seed2 = 15151 + i * 89;

let linestring = generate_random_linestring(seed1, 3 + (i % 3) as usize); // 3-5 points
let polygon = generate_random_polygon(seed2, 4 + (i % 3) as usize); // 4-6 sides
// let linestring = generate_random_linestring(seed1, 3 + (i % 3) as usize); // 3-5 points
// let polygon = generate_random_polygon(seed2, 4 + (i % 3) as usize); // 4-6 sides

let concrete_dist = Euclidean.distance(&linestring, &polygon);
let generic_dist = distance_linestring_to_polygon_generic(&linestring, &polygon);
// let concrete_dist = Euclidean.distance(&linestring, &polygon);
// let generic_dist = distance_linestring_to_polygon_generic(&linestring, &polygon);

assert_relative_eq!(
concrete_dist,
generic_dist,
epsilon = 1e-8,
max_relative = 1e-8
);
}
}
// assert_relative_eq!(
// concrete_dist,
// generic_dist,
// epsilon = 1e-8,
// max_relative = 1e-8
// );
// }
// }

#[test]
fn test_random_symmetry_properties() {
Expand Down
49 changes: 31 additions & 18 deletions rust/sedona-geo/src/st_concavehull.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use arrow_array::builder::BinaryBuilder;
use datafusion_common::error::Result;
use datafusion_common::{cast::as_float64_array, DataFusionError};
use datafusion_expr::ColumnarValue;
use geo::concave_hull::ConcaveHullOptions;
use geo::{ConcaveHull, CoordsIter, Geometry, GeometryCollection, Point, Polygon};
use geo_traits::to_geo::{ToGeoGeometry, ToGeoPoint};
use geo_traits::{GeometryCollectionTrait, GeometryTrait, MultiPointTrait, PointTrait};
Expand All @@ -41,6 +42,10 @@ use crate::to_geo::item_to_geometry;
/// Geo returns a Polygon for every concave hull computation
/// whereas the Geos implementation returns a MultiPolygon for
/// certain geometries concave hull computation.
///
/// Note that this does *not* match the PostGIS implementation.
/// In particular, the GEOS/PostGIS "pctconvex" parameter, which extends
/// from 0..1 does not match this kernel's "concavity" (0..Infinity).
Comment on lines +45 to +48
Copy link
Copy Markdown
Member Author

@paleolimbot paleolimbot Jan 30, 2026

Choose a reason for hiding this comment

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

The Concave Hull algorithm is completely new, but its old output didn't match either. This kernel isn't enabled by default for this reason, but I added this note because in reading the docs it seems that the parameter doesn't carry the same meaning. If we did expose this we might have to give it another name.

pub fn st_concavehull_impl() -> Vec<ScalarKernelRef> {
ItemCrsKernel::wrap_impl(STConcaveHull {})
}
Expand Down Expand Up @@ -84,7 +89,11 @@ fn invoke_batch_impl(arg_types: &[SedonaType], args: &[ColumnarValue]) -> Result
executor.execute_wkb_void(|maybe_wkb| {
match (maybe_wkb, pct_convex_iter.next().unwrap()) {
(Some(wkb), Some(pct_convex)) => {
invoke_scalar(&wkb, pct_convex, &mut builder)?;
let options = ConcaveHullOptions {
concavity: pct_convex,
..Default::default()
};
invoke_scalar(&wkb, options, &mut builder)?;
builder.append_value([]);
}
_ => builder.append_null(),
Expand All @@ -96,13 +105,17 @@ fn invoke_batch_impl(arg_types: &[SedonaType], args: &[ColumnarValue]) -> Result
executor.finish(Arc::new(builder.finish()))
}

fn invoke_scalar(geom: &Wkb, pct_convex: f64, writer: &mut impl std::io::Write) -> Result<()> {
fn invoke_scalar(
geom: &Wkb,
options: ConcaveHullOptions<f64>,
writer: &mut impl std::io::Write,
) -> Result<()> {
if is_empty::is_geometry_empty(geom).map_err(|e| DataFusionError::Execution(e.to_string()))? {
write_geometry(writer, &Polygon::<f64>::empty(), &write_opts())
.map_err(|e| DataFusionError::Execution(e.to_string()))?;
return Ok(());
}
compute_and_write_hull(&normalize_geometry(geom)?, pct_convex, writer)
compute_and_write_hull(&normalize_geometry(geom)?, options, writer)
}

fn write_opts() -> WriteOptions {
Expand Down Expand Up @@ -146,7 +159,7 @@ fn normalize_geometry(geom: &Wkb) -> Result<Geometry> {

fn compute_and_write_hull(
geom: &Geometry,
pct_convex: f64,
options: ConcaveHullOptions<f64>,
writer: &mut impl std::io::Write,
) -> Result<()> {
match geom.as_type() {
Expand All @@ -160,27 +173,27 @@ fn compute_and_write_hull(
.copied()
.collect::<Vec<Point>>(),
)
.concave_hull(pct_convex);
.concave_hull_with_options(options);
write_concave_hull(writer, hull)?;
}

geo_traits::GeometryType::LineString(ls) => {
let hull = ls.concave_hull(pct_convex);
let hull = ls.concave_hull_with_options(options);
write_concave_hull(writer, hull)?;
}

geo_traits::GeometryType::Polygon(pgn) => {
let hull = pgn.concave_hull(pct_convex);
let hull = pgn.concave_hull_with_options(options);
write_concave_hull(writer, hull)?;
}

geo_traits::GeometryType::MultiLineString(mls) => {
let hull = mls.concave_hull(pct_convex);
let hull = mls.concave_hull_with_options(options);
write_concave_hull(writer, hull)?;
}

geo_traits::GeometryType::MultiPolygon(mpgn) => {
let hull = mpgn.concave_hull(pct_convex);
let hull = mpgn.concave_hull_with_options(options);
write_concave_hull(writer, hull)?;
}

Expand All @@ -194,7 +207,7 @@ fn compute_and_write_hull(
coords.into_iter().map(geo_types::Point::from).collect(),
);

let hull = multi_point.concave_hull(pct_convex);
let hull = multi_point.concave_hull_with_options(options);
write_concave_hull(writer, hull)?;
}

Expand Down Expand Up @@ -327,11 +340,11 @@ mod tests {
),
(
"MULTIPOINT ((0 0), (10 0), (0 10), (10 10), (5 5))",
"POLYGON ((10 0, 10 10, 0 10, 0 0, 5 5, 10 0))",
"POLYGON ((10 0, 10 10, 0 10, 0 0, 10 0))",
),
(
"MULTILINESTRING ((10 10, 20 20, 10 40), (40 40, 30 30, 40 20, 30 10))",
"POLYGON ((20 20, 10 40, 30 30, 40 40, 40 20, 30 10, 10 10, 20 20))",
"POLYGON ((30 10, 40 20, 40 40, 10 40, 10 10, 30 10))",
),
(
"MULTIPOLYGON (((2 2, 2 5, 5 5, 5 2, 2 2)), ((6 3, 8 3, 8 1, 6 1, 6 3)))",
Expand Down Expand Up @@ -359,7 +372,7 @@ mod tests {
GEOMETRYCOLLECTION (LINESTRING(6 6,7 7), POLYGON((8 8,9 9,10 10,8 8)))\
)\
)",
"POLYGON ((10 10, 1 1, 3 3, 3 3, 4 4, 5 5, 8 8, 9 9, 10 10))",
"POLYGON ((10 10, 1 1, 10 10))",
),
];

Expand Down Expand Up @@ -418,12 +431,12 @@ mod tests {
(
"MULTILINESTRING ((50 150, 50 200), (50 50, 50 100))",
0.1,
"POLYGON ((50 200, 50 50, 50 100, 50 150, 50 200))",
"POLYGON ((50 200, 50 50, 50 200))",
),
(
"MULTILINESTRING ((50 150, 50 200), (50 50, 50 100))",
0.2,
"POLYGON ((50 200, 50 50, 50 100, 50 150, 50 200))",
"POLYGON ((50 200, 50 50, 50 200))",
),
// Test MULTIPOLYGON with different pctconvex values
(
Expand All @@ -436,22 +449,22 @@ mod tests {
"MULTIPOLYGON (((26 125, 26 200, 126 200, 126 125, 26 125 ),\
( 51 150, 101 150, 76 175, 51 150 )), (( 151 100, 151 200, 176 175, 151 100 )))",
0.4,
"POLYGON((151 100,176 175,151 200,26 200,26 125,151 100))"
"POLYGON ((151 100, 176 175, 151 200, 126 200, 26 200, 26 125, 126 125, 151 100))"
),
// Test GEOMETRYCOLLECTION with different pctconvex values
(
"GEOMETRYCOLLECTION(LINESTRING(1 1,2 2), \
GEOMETRYCOLLECTION(POLYGON((3 3,4 4,5 5,3 3)), \
GEOMETRYCOLLECTION(LINESTRING(6 6,7 7), POLYGON((8 8,9 9,10 10,8 8)))))",
0.1,
"POLYGON ((10 10, 1 1, 3 3, 3 3, 4 4, 5 5, 8 8, 9 9, 10 10))"
"POLYGON ((10 10, 1 1, 10 10))"
),
(
"GEOMETRYCOLLECTION(LINESTRING(1 1,2 2), \
GEOMETRYCOLLECTION(POLYGON((3 3,4 4,5 5,3 3)), \
GEOMETRYCOLLECTION(LINESTRING(6 6,7 7), POLYGON((8 8,9 9,10 10,8 8)))))",
0.6,
"POLYGON ((10 10, 1 1, 3 3, 3 3, 4 4, 5 5, 8 8, 9 9, 10 10))"
"POLYGON ((10 10, 1 1, 10 10))"
),
];

Expand Down
Loading