Keep a group set aside actually set aside
"Not interested" hid a group, and the next Regroup brought it straight back. Reclustering anchors the faces the user has ruled on so a pass cannot move them. It took *confirmations* as the only kind of ruling — but setting a group aside is a ruling too, and the faces it covers are only ever suggestions. So an ignored group's faces entered clustering loose, regrouped into a fresh person that carried no ignore flag, and reappeared in the rail. The original group was left behind holding nothing, hidden and empty. Anchoring them on `ignored` as well as on `confirmed` fixes it, and does one better: a face indexed later that matches a group which was set aside now merges *into* it, so a stranger photographed again stays set aside instead of arriving as somebody new. That is the case that would otherwise have made the feature feel like it only half worked. Three tests, and the first fails without the change — it reports the group coming back with its two faces while the original sits ignored and empty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+150
-1
@@ -505,9 +505,22 @@ pub fn recluster(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Which faces the user has already ruled on, so they enter as anchors.
|
// Which faces the user has already ruled on, so they enter as anchors.
|
||||||
|
//
|
||||||
|
// Two kinds of ruling, and the second is easy to miss. A *confirmation* is
|
||||||
|
// the obvious one. But setting a person aside is a ruling too, and the
|
||||||
|
// faces it covers are only ever suggestions — so anchoring confirmations
|
||||||
|
// alone left every ignored group's faces loose, and the next Regroup
|
||||||
|
// scattered them into fresh unnamed groups that were not ignored. The
|
||||||
|
// strangers came straight back, which is the feature not working at all.
|
||||||
|
//
|
||||||
|
// Anchoring them keeps them where the user put them, and does one better:
|
||||||
|
// a newly indexed face similar to a group that was set aside merges *into*
|
||||||
|
// it, so a stranger photographed again stays set aside instead of
|
||||||
|
// reappearing as somebody new.
|
||||||
let mut confirmed = std::collections::HashMap::new();
|
let mut confirmed = std::collections::HashMap::new();
|
||||||
for p in faces::people(conn)? {
|
for p in faces::people(conn)? {
|
||||||
for f in faces::for_person(conn, p.id, false)? {
|
// Suggestions included exactly when the group was set aside.
|
||||||
|
for f in faces::for_person(conn, p.id, p.ignored)? {
|
||||||
confirmed.insert(f.id, p.id);
|
confirmed.insert(f.id, p.id);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -1055,4 +1068,140 @@ mod tests {
|
|||||||
assert!((rgb[2] - 0.0).abs() < 1e-6);
|
assert!((rgb[2] - 0.0).abs() < 1e-6);
|
||||||
assert!(rgb[3..6].iter().all(|&v| v == 0.0));
|
assert!(rgb[3..6].iter().all(|&v| v == 0.0));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── setting a group aside has to survive regrouping ───────────────────
|
||||||
|
|
||||||
|
const TEST_MODEL: &str = "w600k_mbf";
|
||||||
|
|
||||||
|
/// A catalog holding `n` images and nothing else.
|
||||||
|
fn catalog_with(n: usize) -> Catalog {
|
||||||
|
let c = Catalog::in_memory().unwrap();
|
||||||
|
let conn = c.connection();
|
||||||
|
conn.execute(
|
||||||
|
"INSERT OR IGNORE INTO roots(id, kind, label) VALUES (1, 'local', 'lib')",
|
||||||
|
[],
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
for i in 0..n {
|
||||||
|
conn.execute(
|
||||||
|
&format!(
|
||||||
|
"INSERT INTO images(id, root_id, source_ref, added_at)
|
||||||
|
VALUES ({}, 1, 'IMG_{i}.CR3', 0)",
|
||||||
|
i + 1
|
||||||
|
),
|
||||||
|
[],
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
c
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A unit embedding pointing at `identity`, `cosine` of the way there.
|
||||||
|
fn embedding(identity: usize, cosine: f32) -> Vec<u8> {
|
||||||
|
let mut v = Box::new([0.0_f32; dr_face::EMBEDDING_DIM]);
|
||||||
|
v[identity * 2] = cosine;
|
||||||
|
v[identity * 2 + 1] = (1.0 - cosine * cosine).max(0.0).sqrt();
|
||||||
|
dr_face::Embedding {
|
||||||
|
model: ModelId::new(TEST_MODEL.to_string()),
|
||||||
|
v,
|
||||||
|
}
|
||||||
|
.to_f16_bytes()
|
||||||
|
}
|
||||||
|
|
||||||
|
fn put_face(catalog: &Catalog, image: u64, identity: usize, cosine: f32) {
|
||||||
|
let f = DetectedFace {
|
||||||
|
x: 0.1,
|
||||||
|
y: 0.1,
|
||||||
|
w: 0.2,
|
||||||
|
h: 0.2,
|
||||||
|
landmarks: [(0.0, 0.0); 5],
|
||||||
|
confidence: 0.9,
|
||||||
|
embedding: embedding(identity, cosine),
|
||||||
|
crop_px: 150.0,
|
||||||
|
model_id: TEST_MODEL.to_string(),
|
||||||
|
crop: Vec::new(),
|
||||||
|
};
|
||||||
|
faces::record_detections(
|
||||||
|
catalog.connection(),
|
||||||
|
ImageId(image),
|
||||||
|
TEST_MODEL,
|
||||||
|
1024,
|
||||||
|
std::slice::from_ref(&f),
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The bug this pins: "not interested" only ever covered *suggested* faces,
|
||||||
|
/// and reclustering anchored confirmations alone. So the next Regroup cut
|
||||||
|
/// the ignored group's faces loose, built fresh unnamed groups out of them,
|
||||||
|
/// and every stranger the user had dismissed came straight back.
|
||||||
|
#[test]
|
||||||
|
fn a_group_set_aside_does_not_come_back_on_the_next_regroup() {
|
||||||
|
let catalog = catalog_with(3);
|
||||||
|
put_face(&catalog, 1, 0, 1.0);
|
||||||
|
put_face(&catalog, 2, 0, 0.99);
|
||||||
|
|
||||||
|
recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap();
|
||||||
|
let people = faces::people(catalog.connection()).unwrap();
|
||||||
|
assert_eq!(people.len(), 1, "the two faces should have grouped");
|
||||||
|
let stranger = people[0].id;
|
||||||
|
assert_eq!(people[0].suggested_faces, 2);
|
||||||
|
|
||||||
|
faces::set_ignored(catalog.connection(), stranger, true).unwrap();
|
||||||
|
|
||||||
|
recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap();
|
||||||
|
|
||||||
|
let after = faces::people(catalog.connection()).unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
after.len(),
|
||||||
|
1,
|
||||||
|
"regrouping resurrected the group that was set aside: {after:?}"
|
||||||
|
);
|
||||||
|
assert_eq!(after[0].id, stranger);
|
||||||
|
assert!(after[0].ignored, "the group stopped being set aside");
|
||||||
|
assert_eq!(
|
||||||
|
after[0].suggested_faces, 2,
|
||||||
|
"the faces left the group they were set aside in"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// And it holds as the library grows: a stranger photographed again joins
|
||||||
|
/// the group that was set aside rather than arriving as somebody new.
|
||||||
|
#[test]
|
||||||
|
fn a_new_face_joins_the_group_it_matches_even_when_that_group_is_set_aside() {
|
||||||
|
let catalog = catalog_with(3);
|
||||||
|
put_face(&catalog, 1, 0, 1.0);
|
||||||
|
put_face(&catalog, 2, 0, 0.99);
|
||||||
|
|
||||||
|
recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap();
|
||||||
|
let stranger = faces::people(catalog.connection()).unwrap()[0].id;
|
||||||
|
faces::set_ignored(catalog.connection(), stranger, true).unwrap();
|
||||||
|
|
||||||
|
// The same person turns up in a third photograph.
|
||||||
|
put_face(&catalog, 3, 0, 0.98);
|
||||||
|
recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap();
|
||||||
|
|
||||||
|
let after = faces::people(catalog.connection()).unwrap();
|
||||||
|
assert_eq!(after.len(), 1, "a new face made a second group: {after:?}");
|
||||||
|
assert!(after[0].ignored);
|
||||||
|
assert_eq!(after[0].suggested_faces, 3);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The other half of the promise: bringing them back really does.
|
||||||
|
#[test]
|
||||||
|
fn bringing_a_group_back_makes_it_ordinary_again() {
|
||||||
|
let catalog = catalog_with(3);
|
||||||
|
put_face(&catalog, 1, 0, 1.0);
|
||||||
|
put_face(&catalog, 2, 0, 0.99);
|
||||||
|
recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap();
|
||||||
|
|
||||||
|
let id = faces::people(catalog.connection()).unwrap()[0].id;
|
||||||
|
faces::set_ignored(catalog.connection(), id, true).unwrap();
|
||||||
|
faces::set_ignored(catalog.connection(), id, false).unwrap();
|
||||||
|
|
||||||
|
recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap();
|
||||||
|
let after = faces::people(catalog.connection()).unwrap();
|
||||||
|
assert_eq!(after.len(), 1);
|
||||||
|
assert!(!after[0].ignored);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user