Skip to content

Commit 8251ad0

Browse files
CommanderStormclaudepre-commit-ci[bot]
authored
fix(rust): gate trigram-based shared-dict merging on corpus size (#1505)
Fixes shared dictionary grouping inflation on datasets with many high-cardinality string columns (e.g. OSM hiking relations with `@id`, `relation_ids`, `relation_lengths_m`). `max(exact, trigram) > 0.075` merges columns that share character trigrams (digits, URLs) despite sharing zero actual values. Through transitive union-find closure, nearly all string columns get merged into one giant dictionary. **Fix**: After forming groups with the existing aggressive heuristic, each group whose combined corpus exceeds 100 KB is checked for actual cross-column value deduplication. Groups with <5% dedup are dissolved. Tjios means that existing OMT groupings are preserved, since they have genuine value overlap (multilingual name translations share most values across `name`, `name:latin`, `name_de`, `name_en`, `name_int`): | Tile | Before | After | Change | |---|---:|---:|---:| | OMT z14 (23K features) | 378.1 KB | 378.1 KB | **+0.0%** | | OMT z7 | 178.1 KB | 178.1 KB | **+0.0%** | | Hiking z0 | 180.0 KB | 155.5 KB | **−13.6%** | | Hiking z2 | 678.9 KB | 555.2 KB | **−18.2%** | | Hiking z4 (86K features) | 5.2 MB | 4.4 MB | **−14.8%** | | Hiking z6 | 3.4 MB | 2.9 MB | **−16.0%** | | Hiking z8 | 543.0 KB | 445.7 KB | **−17.9%** | | Hiking z10 | 173.6 KB | 138.6 KB | **−20.2%** | --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent 13cf6cd commit 8251ad0

2 files changed

Lines changed: 112 additions & 2 deletions

File tree

rust/mlt-core/src/encoder/property/shared_dict.rs

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,18 @@ const MINHASH_PERMUTATIONS: usize = 128;
2828
/// grouped into a single shared dictionary.
2929
const MINHASH_SIMILARITY_THRESHOLD: f64 = 0.075;
3030

31+
/// Groups whose sum of per-column unique-corpus bytes exceeds this are validated via [`group_is_beneficial`].
32+
/// Smaller groups are kept unconditionally.
33+
const VALIDATE_CORPUS_THRESHOLD: usize = 100_000;
34+
35+
/// Minimum dedup ratio (`1 − union/sum_individual`) for a validated group to be retained.
36+
const MIN_DEDUP_RATIO: f64 = 0.05;
37+
3138
struct StringProfile<'a> {
3239
col_idx: usize,
3340
name: &'a str,
41+
/// Sorted, deduplicated values for [`group_is_beneficial`].
42+
unique_values: Vec<&'a str>,
3443
/// `MinHash` over exact string values.
3544
exact_hashes: Vec<u64>,
3645
/// `MinHash` over byte trigrams (empty when all strings are shorter than 3 bytes).
@@ -61,7 +70,7 @@ impl TileLayer {
6170
.iter()
6271
.enumerate()
6372
.filter_map(|(col_idx, name)| {
64-
let vals: Vec<&str> = self
73+
let mut vals: Vec<&str> = self
6574
.features()
6675
.iter()
6776
.filter_map(|f| match f.properties().get(col_idx) {
@@ -72,6 +81,8 @@ impl TileLayer {
7281
if vals.is_empty() {
7382
return None;
7483
}
84+
vals.sort_unstable();
85+
vals.dedup();
7586
let exact_hashes = exact_mh.get_min_hashes(vals.iter().copied());
7687
let trigrams: Vec<[u8; 3]> = vals
7788
.iter()
@@ -85,6 +96,7 @@ impl TileLayer {
8596
Some(StringProfile {
8697
col_idx,
8798
name,
99+
unique_values: vals,
88100
exact_hashes,
89101
trigram_hashes,
90102
})
@@ -120,6 +132,34 @@ fn minhash_similarity(a: &[u64], b: &[u64]) -> f64 {
120132
matches as f64 / a.len() as f64
121133
}
122134

135+
/// Check whether a proposed shared-dictionary group has enough cross-column
136+
/// value deduplication to justify the combined dictionary.
137+
///
138+
/// Groups below [`VALIDATE_CORPUS_THRESHOLD`] are always kept. Larger groups
139+
/// must achieve at least [`MIN_DEDUP_RATIO`] dedup savings.
140+
#[allow(clippy::cast_precision_loss)]
141+
fn group_is_beneficial(group: &[StringProfile<'_>]) -> bool {
142+
let sum_individual: usize = group
143+
.iter()
144+
.map(|p| p.unique_values.iter().map(|s| s.len()).sum::<usize>())
145+
.sum();
146+
147+
if sum_individual <= VALIDATE_CORPUS_THRESHOLD {
148+
return true;
149+
}
150+
151+
let mut all_values: Vec<&str> = group
152+
.iter()
153+
.flat_map(|p| p.unique_values.iter().copied())
154+
.collect();
155+
all_values.sort_unstable();
156+
all_values.dedup();
157+
let union_bytes: usize = all_values.iter().map(|s| s.len()).sum();
158+
159+
let dedup_savings = sum_individual.saturating_sub(union_bytes);
160+
dedup_savings as f64 / sum_individual as f64 >= MIN_DEDUP_RATIO
161+
}
162+
123163
fn cluster_by_similarity(profiles: Vec<StringProfile<'_>>) -> Vec<Vec<StringProfile<'_>>> {
124164
if profiles.is_empty() {
125165
return Vec::new();
@@ -146,7 +186,7 @@ fn cluster_by_similarity(profiles: Vec<StringProfile<'_>>) -> Vec<Vec<StringProf
146186
let mut groups: Vec<Vec<StringProfile<'_>>> = groups_map
147187
.into_values()
148188
.filter_map(|mut v| {
149-
if v.len() >= 2 {
189+
if v.len() >= 2 && group_is_beneficial(&v) {
150190
v.sort_unstable_by_key(|p| p.col_idx);
151191
Some(v)
152192
} else {

rust/mlt-core/src/encoder/property/tests.rs

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,6 +1295,76 @@ fn mixed_scalars_and_grouped_strings() {
12951295
assert_eq!(res.properties[3].stats.shared_dict(), SharedDictRole::None);
12961296
}
12971297

1298+
#[test]
1299+
fn high_cardinality_digit_columns_not_grouped() {
1300+
let n = 5000;
1301+
let col_a: Vec<PropValue> = (0..n)
1302+
.map(|i| PropValue::Str(Some(format!("way/{}", 100_000 + i))))
1303+
.collect();
1304+
let col_b: Vec<PropValue> = (0..n)
1305+
.map(|i| PropValue::Str(Some(format!("{},{}", 200_000 + i, 300_000 + i))))
1306+
.collect();
1307+
let col_c: Vec<PropValue> = (0..n)
1308+
.map(|i| {
1309+
PropValue::Str(Some(format!(
1310+
"{}:{}|{}:{}",
1311+
200_000 + i,
1312+
4 + i,
1313+
300_000 + i,
1314+
8 + i
1315+
)))
1316+
})
1317+
.collect();
1318+
1319+
let tile = tile_from_cols(&[
1320+
("@id", col_a),
1321+
("relation_ids", col_b),
1322+
("relation_data", col_c),
1323+
]);
1324+
let res = tile.analyze(true).unwrap();
1325+
1326+
for (idx, prop) in res.properties.iter().enumerate() {
1327+
assert_eq!(
1328+
prop.stats.shared_dict(),
1329+
SharedDictRole::None,
1330+
"column {idx} should not be in a shared dict"
1331+
);
1332+
}
1333+
}
1334+
1335+
#[test]
1336+
fn overlapping_name_columns_stay_grouped() {
1337+
let n = 500;
1338+
let shared_names: Vec<String> = (0..n).map(|i| format!("Place_{i}")).collect();
1339+
let col_name: Vec<PropValue> = shared_names
1340+
.iter()
1341+
.map(|s| PropValue::Str(Some(s.clone())))
1342+
.collect();
1343+
let col_en: Vec<PropValue> = shared_names
1344+
.iter()
1345+
.enumerate()
1346+
.map(|(i, s)| {
1347+
if i % 10 == 0 {
1348+
PropValue::Str(Some(format!("{s} (EN)")))
1349+
} else {
1350+
PropValue::Str(Some(s.clone()))
1351+
}
1352+
})
1353+
.collect();
1354+
1355+
let tile = tile_from_cols(&[("name", col_name), ("name:en", col_en)]);
1356+
let res = tile.analyze(true).unwrap();
1357+
1358+
assert_eq!(
1359+
res.properties[0].stats.shared_dict(),
1360+
SharedDictRole::Owner("name".to_string())
1361+
);
1362+
assert_eq!(
1363+
res.properties[1].stats.shared_dict(),
1364+
SharedDictRole::Member(0)
1365+
);
1366+
}
1367+
12981368
#[test]
12991369
fn encode_with_explicit_encoder_works() {
13001370
let props = vec![StagedProperty::u32("id", (1_000u32..2_000).collect())];

0 commit comments

Comments
 (0)