feat(survey): evaluate LHS points in greedy-nearest order#428
Closed
vsbuffalo wants to merge 1 commit into
Closed
Conversation
Replace the flat index-order `par_iter` sweep with a batched greedy schedule: seed a coarse spread of points, then each round evaluate the unevaluated points nearest — in transform-normalized parameter space — to the current best-loglik point, a batch at a time so every core stays busy. The payoff is a live signal. The progress bar's best-loglik metric now climbs fast and plateaus early when the box holds a basin, which is an at-a-glance read on whether the survey bounds are placed well, and the good region is the part that fills in first — easier to watch, and quick to notice a mis-placed box. This reorders WHEN points run, never WHICH: every point is still evaluated (no early stop), each point's result is keyed on (seed, point_id) via `derive_point_seed` / `mix_cell_seed` rather than a shared RNG stream, and the final sort by (loglik desc, point_id asc) is order-independent. So `landscape.tsv` is byte-identical to the old sweep — no golden, expected-output, or CAS-identity impact. The distance metric reuses `EstimatedParam::to_transformed`, so points on log/logit params are compared on the same scale the sampler and landscape geometry live on; it only orders evaluation and never touches a loglik. `survey_greedy_order_invariant.rs` pins the invariant from outside: the same survey at `--parallel 1` vs `--parallel 4` walks two different greedy orders and must produce byte-identical `landscape.tsv`. If a future change makes a point's result order-dependent, it goes red.
Owner
Author
|
Closing — wrong target. This applied greedy ordering to |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
camdl surveynow evaluates its LHS points in a greedy-nearest order — seed a coarse spread, then each round evaluate the unevaluated points closest (in transform-normalized parameter space) to the current best-loglik point, a batch at a time so every core stays busy.Why
The progress bar's best-loglik metric climbs fast and plateaus early when the box actually holds a basin — an at-a-glance read on whether the survey bounds are placed well — and the good region is the part that fills in first. Easier to watch, and quick to notice a mis-placed box. (No early-stop — deliberately. Every point is still evaluated; a survey's job is the global picture, and best-first + early-stop could bias toward the first basin and miss a distant better mode.)
Byte-identical output
This reorders when points run, never which:
(seed, point_id)(derive_point_seed/mix_cell_seed), not a shared RNG stream, so it's independent of evaluation order;(loglik desc, point_id asc)is order-independent.So
landscape.tsvis byte-identical to the old index-order sweep — no golden / expected-output / CAS-identity impact. The greedy distance metric reusesEstimatedParam::to_transformed(log/logit-aware) and only orders evaluation; it never touches a loglik.Test
survey_greedy_order_invariant.rspins it from outside: the same survey run at--parallel 1vs--parallel 4walks two different greedy orders and must produce byte-identicallandscape.tsv. Goes red if a future change makes a point's result order-dependent.Green locally: the new invariant test, plus
survey_roundtripandfit_survey_denominator(no regression).