Skip to content

T-208/T-211: random_constrained_tree() and impose_constraint() fixes#229

Merged
ms609 merged 6 commits intocpp-searchfrom
feature/random-constrained-tree
Mar 25, 2026
Merged

T-208/T-211: random_constrained_tree() and impose_constraint() fixes#229
ms609 merged 6 commits intocpp-searchfrom
feature/random-constrained-tree

Conversation

@ms609
Copy link
Owner

@ms609 ms609 commented Mar 25, 2026

Summary

Implements random_constrained_tree() and fixes three bugs in impose_constraint().

New: random_constrained_tree() (T-208)

Constraint-aware random topology builder. Orders constraint splits smallest→largest,
assigns tips to tightest enclosing split, randomly resolves each split's children
into a binary subtree, then wires root level. Replaces the Wagner tree fallback
that T-214 introduced as a workaround.

Fix: impose_constraint() bugs (T-211)

Three interconnected bugs prevented constraint repair from working correctly:

Bug Fix
Bail-out threshold n_tip/4 too aggressive Raised to n_tip
Return 0 for both 'no violations' and 'bailed out' Return -1 on bail-out
spr_clip() can't detach root children New topology_spr() helper (65 lines)

Testing

  • GHA run 23557186264: 0 FAIL, 10927 PASS on both Ubuntu and Windows
  • Run marked FAIL due to pre-existing doc warnings (SearchControl.Rd, PrepareDataProfile.Rd) — unrelated to this PR
  • 942 constraint tests pass (90 impose-constraint incl. 2 new root-child tests, 806 constraint-multi, 22 constraint-small, 24 random-constrained)
  • 308 simplify/driven/sector tests pass — no regressions

Agent A (Claude Sonnet 4).

ms609 added 5 commits March 25, 2026 16:55
…ilder

Build random binary tree that satisfies topological constraints by:
1. Ordering constraint splits smallest-to-largest
2. Assigning each tip to its tightest enclosing split
3. Bottom-up: randomly resolve each split's children into binary subtree
4. Wire root-level items (unconstrained tips + top-level splits)

Replaces Wagner fallback (T-214 workaround) for RANDOM_TREE strategy,
restoring uniform random topology sampling diversity under constraints.

Tests: 916 constraint + 24 random-constrained + 373 simplify/driven pass.
Three issues in impose_one_pass / impose_constraint:

1. Bail-out threshold n_tip/4 was too aggressive. For n_tip=5 the
   threshold was 1, so any split requiring 2+ moves caused an immediate
   bail-out before making any repairs. Raised to n_tip.

2. impose_one_pass returned 0 both for 'no violations' and 'bailed out',
   so impose_constraint couldn't distinguish the two. Now returns -1 on
   bail-out, allowing the caller to know the repair may be incomplete.

3. Documented the root-child limitation: spr_clip() doesn't fully handle
   root children, so impose_one_pass skips them. The fuse path's
   post-repair verification guard (T-214) catches these cases.

Tests: 940 constraint + 308 simplify/driven/sector pass.
spr_clip() can't detach root children (root is its own parent, so the
bypass logic fails). All search callers skip root children, but
impose_constraint needs them for constraint repair.

New topology_spr() helper in ts_constraint.cpp handles the root-child
case by absorbing the sibling into root and repurposing it as the
insertion node. No changes to spr_clip or any search caller.

Also removes the build_postorder call that was missing between
individual moves within impose_one_pass — each topology_spr is now
followed by a postorder rebuild so edge enumeration stays valid.

Tests: 942 constraint (90 impose-constraint, incl 2 new root-child
tests) + 308 simplify/driven/sector pass.
@ms609 ms609 force-pushed the feature/random-constrained-tree branch from d523c99 to 8cd3ac7 Compare March 25, 2026 19:31
@ms609 ms609 merged commit e6ba08c into cpp-search Mar 25, 2026
2 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant