dtourolle/DarkRoom
1
0
Fork 0
Code Issues 39 Pull Requests Actions Packages Projects Releases 20 Wiki Activity

Put the grouping dials where the regrouping is

Browse Source 
The merge probability was `dr_face`'s constant and the smallest group was
a bare `< 2` in the clustering pass. Both were tuned on one library —
1,813 faces of one photographer's family — and the quantity they optimise
is a property of the population, not of the model. A household at close
family resemblance and two thousand strangers at a wedding want different
answers, and neither of them is the reference library. The doc comment
already conceded the point and pointed at `face_index --tune`; a
photographer does not have a terminal.

So they are `FaceSettings` now, saved per device beside the cache budgets
and edited from the People screen — beside the Regroup button that
applies them and the rail that shows what they did, because a value
changed three screens away from its effect is one nobody can tune.

Moving them is safe by construction, which is why nothing asks for
confirmation: a regroup writes only the suggested half, and
confirmations, names and ignores enter as anchors and come back
unchanged. The smallest-group rule is applied only to groups the system
invented — a group the user named or set aside survives it whatever its
size, because a display preference does not overrule a judgement.

**Withdrawal, without which the setting does nothing visible.** Raising
the smallest group stops the pass creating small groups; it does not
remove the ones a previous pass made, because those still hold their
suggestions, so they are not empty, so the prune leaves them. The pass
now releases every unanchored face it did not place before pruning.

And a dial you cannot see the effect of is not a dial. "What would this
do?" runs the same population through the clusterer without opening a
transaction and reports groups, faces grouped and largest group — one row
of `--tune`'s table, on the user's own library, on a worker thread. The
line leads with the group count because that is the number that says
which side of the right setting you are on: it climbs as fragments are
gathered into people and falls as separate people start being welded,
while the grouped-face count rises straight through both.

The preview parks its poll timer in a slot of its own. A preview and a
regroup are allowed to be in flight together, and sharing the sweep's
single slot would have the second to start drop the first's timer —
visible as a Regroup that finished on its worker and never said so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-08-29 23:18:46 +02:00
co-authored by Claude Opus 5
parent e465ff0c80
commit d0b671d4db
10 changed files with 883 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/faces.rs
+347 -75
View File
@@ -30,6 +30,7 @@ use dr_catalog::faces::{self, DetectedFace};
use dr_catalog::Catalog;
use dr_face::{align, Calibration, DetectOptions, Detection, Detector, Embedder, ModelId};
use dr_thumbs::{ThumbSize, ThumbStore};
use dr_types::settings::FaceSettings;
use dr_types::ImageId;
/// The tier faces are found on. See the module note.
@@ -476,6 +477,148 @@ pub fn spawn_store_face_sweep(
rx
}
/// Everything a grouping pass reads before it decides anything.
///
/// Split out because two callers need exactly this and must agree on it: the
/// pass that writes, and the preview that reports what the pass *would* do. A
/// preview built from a second, subtly different reading — anchors omitted,
/// say — would answer a question about a library nobody has.
struct Population {
cal: Calibration,
candidates: Vec<dr_face::Candidate>,
/// Catalog ids, parallel to `candidates`.
ids: Vec<faces::FaceId>,
/// Faces the user has ruled on, and who they are. See [`Population::read`].
anchors: std::collections::HashMap<faces::FaceId, faces::PersonId>,
}
impl Population {
/// Takes the catalog and not its connection: `rusqlite` is `dr-catalog`'s
/// dependency and not this crate's, and reaching for the connection type by
/// name here would drag it across a layer that has kept clear of it.
fn read(catalog: &Catalog, model_id: &str) -> Result<Self, dr_catalog::CatalogError> {
let conn = catalog.connection();
// No valid calibration is not a reason to refuse to cluster — it is a
// reason not to *display* a confidence (FR-CULL-9). `Calibration::
// default` is the reference implementation's fitted curve with `valid`
// false, which is a documented operating point rather than an invented
// one.
let cal = faces::calibration(conn, model_id)?
.map(|(c, _)| c)
.unwrap_or_else(Calibration::default);
// Which faces the user has already ruled on, so they enter as anchors.
//
// Anything the user has ruled on anchors, and there are three ways of
// ruling — only the first of which is obvious.
//
// A **confirmation** is the plain case. **Setting a group aside** is one
// too, and the faces it covers are only ever suggestions, so anchoring
// confirmations alone let every ignored group scatter into fresh unnamed
// groups that were not ignored, and the strangers came straight back.
//
// And so is **giving a group a name**. That was the omission that did
// the most damage, because it is silent. Naming a cluster does not
// confirm its faces — they stay suggestions — so the next Regroup cut
// them loose, regrouped them into a brand new person, and left the named
// one holding nothing. `prune_empty_unnamed` will not remove it, because
// it has a name. Name the new group the same thing and it happens again.
// That is how one library came to hold sixteen people called Catherine,
// fourteen of them empty, with her faces split across the two that were
// not.
//
// A name is a judgement about *this group* (FR-CULL-12), exactly as an
// ignore is. Anchoring them all also does one better: a newly indexed
// face that matches a named person now merges *into* them rather than
// arriving as a stranger.
let mut anchors = std::collections::HashMap::new();
for p in faces::people(conn)? {
let ruled_on = p.ignored || !p.name.trim().is_empty();
for f in faces::for_person(conn, p.id, ruled_on)? {
anchors.insert(f.id, p.id);
}
}
let stored = faces::embeddings(conn, model_id)?;
let model = ModelId::new(model_id.to_string());
let mut candidates = Vec::with_capacity(stored.len());
let mut ids = Vec::with_capacity(stored.len());
for (face_id, image_id, blob, crop_px) in stored {
let Some(emb) = dr_face::Embedding::from_f16_bytes(model.clone(), &blob) else {
log::warn!("face {face_id:?} has a malformed embedding, skipped");
continue;
};
candidates.push(dr_face::Candidate {
face: face_id.0,
image: image_id.0,
embedding: emb.v.to_vec(),
crop_px,
confirmed_person: anchors.get(&face_id).map(|p| p.0),
});
ids.push(face_id);
}
Ok(Self {
cal,
candidates,
ids,
anchors,
})
}
}
/// What a grouping pass would produce, without producing it.
///
/// The numbers `dr_face::cluster`'s own tuning table is built from, for one
/// setting rather than ten — because the question a photographer is actually
/// asking of the dials is "what does *my* library look like at this value", and
/// the doc-comment table answers it for a library that is not theirs.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
pub struct GroupingPreview {
/// Faces that went in.
pub faces: usize,
/// Groups that would survive the smallest-group rule.
pub groups: usize,
/// Faces those groups would hold.
pub grouped: usize,
/// The biggest group. The tell for over-merging: it is the number that runs
/// away when the confidence is set too low.
pub largest: usize,
}
/// Report what a grouping pass would do, writing nothing.
///
/// Read-only by construction — it never opens a transaction — which is what
/// makes it safe to run repeatedly while the user moves a slider. Comparing two
/// settings by *applying* both would leave the second one's answer polluted by
/// the first one's suggestions.
pub fn preview_grouping(
catalog: &Catalog,
model_id: &str,
grouping: &FaceSettings,
) -> Result<GroupingPreview, dr_catalog::CatalogError> {
let min_group = grouping.min_group_size.max(1) as usize;
let pop = Population::read(catalog, model_id)?;
if pop.candidates.is_empty() {
return Ok(GroupingPreview::default());
}
let clusters = dr_face::cluster(&pop.candidates, &pop.cal, grouping.merge_probability);
// The same rule the writing pass applies, so the preview and the result
// cannot disagree about what counts as a person.
let kept: Vec<_> = clusters
.iter()
.filter(|c| c.members.len() >= min_group || c.person.is_some())
.collect();
Ok(GroupingPreview {
faces: pop.candidates.len(),
groups: kept.len(),
grouped: kept.iter().map(|c| c.members.len()).sum(),
largest: kept.iter().map(|c| c.members.len()).max().unwrap_or(0),
})
}
/// Group the library's faces into people, writing suggestions.
///
/// Confirmations are never touched: they enter the clusterer as anchors and
@@ -487,72 +630,22 @@ pub fn spawn_store_face_sweep(
pub fn recluster(
catalog: &Catalog,
model_id: &str,
min_probability: f32,
grouping: &FaceSettings,
) -> Result<(usize, usize), dr_catalog::CatalogError> {
let min_probability = grouping.merge_probability;
let min_group = grouping.min_group_size.max(1) as usize;
let conn = catalog.connection();
// No valid calibration is not a reason to refuse to cluster — it is a
// reason not to *display* a confidence (FR-CULL-9). `Calibration::default`
// is the reference implementation's fitted curve with `valid` false, which
// is a documented operating point rather than an invented one.
let cal = faces::calibration(conn, model_id)?
.map(|(c, _)| c)
.unwrap_or_else(Calibration::default);
let stored = faces::embeddings(conn, model_id)?;
if stored.is_empty() {
let Population {
cal,
candidates,
ids,
anchors: confirmed,
} = Population::read(catalog, model_id)?;
if candidates.is_empty() {
return Ok((0, 0));
}
// Which faces the user has already ruled on, so they enter as anchors.
//
// Anything the user has ruled on anchors, and there are three ways of
// ruling — only the first of which is obvious.
//
// A **confirmation** is the plain case. **Setting a group aside** is one
// too, and the faces it covers are only ever suggestions, so anchoring
// confirmations alone let every ignored group scatter into fresh unnamed
// groups that were not ignored, and the strangers came straight back.
//
// And so is **giving a group a name**. That was the omission that did the
// most damage, because it is silent. Naming a cluster does not confirm its
// faces — they stay suggestions — so the next Regroup cut them loose,
// regrouped them into a brand new person, and left the named one holding
// nothing. `prune_empty_unnamed` will not remove it, because it has a name.
// Name the new group the same thing and it happens again. That is how one
// library came to hold sixteen people called Catherine, fourteen of them
// empty, with her faces split across the two that were not.
//
// A name is a judgement about *this group* (FR-CULL-12), exactly as an
// ignore is. Anchoring them all also does one better: a newly indexed face
// that matches a named person now merges *into* them rather than arriving
// as a stranger.
let mut confirmed = std::collections::HashMap::new();
for p in faces::people(conn)? {
let ruled_on = p.ignored || !p.name.trim().is_empty();
for f in faces::for_person(conn, p.id, ruled_on)? {
confirmed.insert(f.id, p.id);
}
}
let model = ModelId::new(model_id.to_string());
let mut candidates = Vec::with_capacity(stored.len());
let mut ids = Vec::with_capacity(stored.len());
for (face_id, image_id, blob, crop_px) in stored {
let Some(emb) = dr_face::Embedding::from_f16_bytes(model.clone(), &blob) else {
log::warn!("face {face_id:?} has a malformed embedding, skipped");
continue;
};
candidates.push(dr_face::Candidate {
face: face_id.0,
image: image_id.0,
embedding: emb.v.to_vec(),
crop_px,
confirmed_person: confirmed.get(&face_id).map(|p| p.0),
});
ids.push(face_id);
}
let dr_face::Grouping {
clusters,
confidence,
@@ -560,10 +653,20 @@ pub fn recluster(
let mut suggested = 0usize;
let mut created = 0usize;
// Every face this pass actually placed. What is *not* in here at the end is
// a face the previous pass had an opinion about and this one does not, and
// it has to be let go — see below.
let mut placed = std::collections::HashSet::with_capacity(ids.len());
for c in &clusters {
// A group of one is not a person. Naming every stray face would fill
// the People view with noise the user then has to dismiss.
if c.members.len() < 2 && c.person.is_none() {
// A group of one is not a person, and the user says how much bigger
// than one it has to be (`FaceSettings::min_group_size`). Naming every
// stray face would fill the People view with noise they then have to
// dismiss one entry at a time.
//
// Only ever applied to a group the system invented. A group with a
// `person` is one the user has already confirmed, named or set aside,
// and a display preference does not overrule a judgement (FR-CULL-12).
if c.members.len() < min_group && c.person.is_none() {
continue;
}
@@ -579,6 +682,7 @@ pub fn recluster(
for &m in &c.members {
let face = ids[m];
placed.insert(face);
if confirmed.contains_key(&face) {
continue;
}
@@ -594,6 +698,31 @@ pub fn recluster(
}
}
// Faces the previous pass placed and this one did not.
//
// Without this the parameters above are only half connected to the screen.
// Raise the smallest group to three and the pass stops *creating* groups of
// two — but last pass's group of two still holds its two suggestions, so it
// is not empty, so the prune below leaves it, and the rail does not change.
// The setting would appear to do nothing until the library was reindexed.
//
// Suggestions are this pass's own output (FR-CULL-10), so withdrawing one it
// no longer stands behind is exactly what it is entitled to do. Anchors are
// skipped by construction: a confirmed, named or ignored face is in
// `confirmed`, enters as an anchor, and comes back out inside a group that
// is never dropped.
let mut released = 0usize;
for face in &ids {
if placed.contains(face) || confirmed.contains_key(face) {
continue;
}
faces::unassign(conn, *face)?;
released += 1;
}
if released > 0 {
log::info!("reclustering released {released} face(s) it no longer groups");
}
// Groups the *previous* pass created that this one left empty. Without
// this, every press of Regroup adds a rail entry per group it no longer
// believes in, and the screen fills with "Unnamed (0 faces)" — which is
@@ -612,6 +741,43 @@ pub fn recluster(
Ok((suggested, created))
}
/// The answer from a preview pass.
#[derive(Debug, Clone, PartialEq)]
pub enum PreviewMessage {
Ready(GroupingPreview),
Failed(String),
}
/// Report what a grouping pass would do, **on a worker thread**.
///
/// Same reasoning as [`spawn_recluster`], and the same cost: a preview is a
/// clustering pass that throws its answer away, so it is exactly as unbounded
/// as the pass it is previewing and exactly as unwelcome on the UI thread.
///
/// Cancellation is dropping the receiver — so moving the slider again while one
/// is in flight abandons it, which is the behaviour a dial with a preview
/// button needs.
pub fn spawn_grouping_preview(
catalog_path: PathBuf,
model_id: String,
grouping: FaceSettings,
) -> Receiver<PreviewMessage> {
let (tx, rx) = std::sync::mpsc::channel();
std::thread::spawn(move || {
let msg = match Catalog::open(&catalog_path) {
Ok(catalog) => match preview_grouping(&catalog, &model_id, &grouping) {
Ok(p) => PreviewMessage::Ready(p),
Err(e) => PreviewMessage::Failed(e.to_string()),
},
Err(e) => PreviewMessage::Failed(format!("cannot open catalog: {e}")),
};
let _ = tx.send(msg);
});
rx
}
/// Progress from a regrouping pass.
#[derive(Debug, Clone, PartialEq)]
pub enum ReclusterMessage {
@@ -643,7 +809,7 @@ pub enum ReclusterMessage {
pub fn spawn_recluster(
catalog_path: PathBuf,
model_id: String,
min_probability: f32,
grouping: FaceSettings,
) -> Receiver<ReclusterMessage> {
let (tx, rx) = std::sync::mpsc::channel();
@@ -667,7 +833,7 @@ pub fn spawn_recluster(
return;
}
let msg = match recluster(&catalog, &model_id, min_probability) {
let msg = match recluster(&catalog, &model_id, &grouping) {
Ok((suggested, created)) => ReclusterMessage::Finished { suggested, created },
Err(e) => ReclusterMessage::Failed(e.to_string()),
};
@@ -1125,7 +1291,7 @@ mod tests {
put_face(&catalog, 1, 0, 1.0);
put_face(&catalog, 2, 0, 0.99);
recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap();
recluster(&catalog, TEST_MODEL, &FaceSettings::default()).unwrap();
let people = faces::people(catalog.connection()).unwrap();
assert_eq!(people.len(), 1, "the two faces should have grouped");
let stranger = people[0].id;
@@ -1133,7 +1299,7 @@ mod tests {
faces::set_ignored(catalog.connection(), stranger, true).unwrap();
recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap();
recluster(&catalog, TEST_MODEL, &FaceSettings::default()).unwrap();
let after = faces::people(catalog.connection()).unwrap();
assert_eq!(
@@ -1157,13 +1323,13 @@ mod tests {
put_face(&catalog, 1, 0, 1.0);
put_face(&catalog, 2, 0, 0.99);
recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap();
recluster(&catalog, TEST_MODEL, &FaceSettings::default()).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();
recluster(&catalog, TEST_MODEL, &FaceSettings::default()).unwrap();
let after = faces::people(catalog.connection()).unwrap();
assert_eq!(after.len(), 1, "a new face made a second group: {after:?}");
@@ -1177,13 +1343,13 @@ mod tests {
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();
recluster(&catalog, TEST_MODEL, &FaceSettings::default()).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();
recluster(&catalog, TEST_MODEL, &FaceSettings::default()).unwrap();
let after = faces::people(catalog.connection()).unwrap();
assert_eq!(after.len(), 1);
assert!(!after[0].ignored);
@@ -1200,7 +1366,7 @@ mod tests {
put_face(&catalog, 1, 0, 1.0);
put_face(&catalog, 2, 0, 0.99);
recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap();
recluster(&catalog, TEST_MODEL, &FaceSettings::default()).unwrap();
let people = faces::people(catalog.connection()).unwrap();
assert_eq!(people.len(), 1);
let her = people[0].id;
@@ -1210,7 +1376,7 @@ mod tests {
// types a name and moves on has done.
faces::rename_person(catalog.connection(), her, "Catherine").unwrap();
recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap();
recluster(&catalog, TEST_MODEL, &FaceSettings::default()).unwrap();
let after = faces::people(catalog.connection()).unwrap();
assert_eq!(
@@ -1233,15 +1399,121 @@ mod tests {
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();
recluster(&catalog, TEST_MODEL, &FaceSettings::default()).unwrap();
let her = faces::people(catalog.connection()).unwrap()[0].id;
faces::rename_person(catalog.connection(), her, "Catherine").unwrap();
put_face(&catalog, 3, 0, 0.98);
recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap();
recluster(&catalog, TEST_MODEL, &FaceSettings::default()).unwrap();
let after = faces::people(catalog.connection()).unwrap();
assert_eq!(after.len(), 1, "a second Catherine appeared: {after:?}");
assert_eq!(after[0].suggested_faces, 3);
}
/// `dr-types` sits below the face engine and cannot name its constant, so
/// it restates the number. This is the thing that stops the two drifting:
/// a settings page marking 0.80 as "default" while the engine had moved to
/// 0.75 would put the reset dot on a value nothing else agreed with.
#[test]
fn the_settings_default_is_the_engines_own_tuned_value() {
assert_eq!(
FaceSettings::default().merge_probability,
dr_face::DEFAULT_MERGE_PROBABILITY
);
}
/// The smallest-group setting has to change what is on the rail, not just
/// what the *next* pass would build. Before the release step in
/// [`recluster`], raising it left the previous pass's small groups sitting
/// there full of suggestions — not empty, so not pruned — and the control
/// looked broken.
#[test]
fn raising_the_smallest_group_takes_the_small_groups_off_the_rail() {
let catalog = catalog_with(5);
// One pair, and one trio: two groups at the default of two.
put_face(&catalog, 1, 0, 1.0);
put_face(&catalog, 2, 0, 0.99);
put_face(&catalog, 3, 1, 1.0);
put_face(&catalog, 4, 1, 0.99);
put_face(&catalog, 5, 1, 0.98);
recluster(&catalog, TEST_MODEL, &FaceSettings::default()).unwrap();
assert_eq!(
faces::people(catalog.connection()).unwrap().len(),
2,
"the two groups should have formed at the default"
);
recluster(
&catalog,
TEST_MODEL,
&FaceSettings {
min_group_size: 3,
..FaceSettings::default()
},
)
.unwrap();
let after = faces::people(catalog.connection()).unwrap();
assert_eq!(
after.len(),
1,
"the pair survived a smallest-group of three: {after:?}"
);
assert_eq!(after[0].suggested_faces, 3, "the trio lost members");
}
/// And the setting does not overrule the user. A group they named is theirs
/// (FR-CULL-12), however few faces it holds.
#[test]
fn a_named_group_survives_a_smallest_group_it_is_under() {
let catalog = catalog_with(3);
put_face(&catalog, 1, 0, 1.0);
put_face(&catalog, 2, 0, 0.99);
recluster(&catalog, TEST_MODEL, &FaceSettings::default()).unwrap();
let her = faces::people(catalog.connection()).unwrap()[0].id;
faces::rename_person(catalog.connection(), her, "Catherine").unwrap();
recluster(
&catalog,
TEST_MODEL,
&FaceSettings {
min_group_size: 6,
..FaceSettings::default()
},
)
.unwrap();
let after = faces::people(catalog.connection()).unwrap();
assert_eq!(after.len(), 1, "Catherine was dropped: {after:?}");
assert_eq!(after[0].name, "Catherine");
assert_eq!(after[0].suggested_faces, 2);
}
/// Down at one, every stray becomes a group of its own — which is what a
/// user hunting for a face the grouping missed has asked for.
#[test]
fn a_smallest_group_of_one_shows_the_strays() {
let catalog = catalog_with(2);
put_face(&catalog, 1, 0, 1.0);
put_face(&catalog, 2, 1, 1.0);
recluster(&catalog, TEST_MODEL, &FaceSettings::default()).unwrap();
assert!(
faces::people(catalog.connection()).unwrap().is_empty(),
"two unrelated faces made a person at the default"
);
recluster(
&catalog,
TEST_MODEL,
&FaceSettings {
min_group_size: 1,
..FaceSettings::default()
},
)
.unwrap();
assert_eq!(faces::people(catalog.connection()).unwrap().len(), 2);
}
}