Repository navigation
feat(api): honour log scale with normal distributions + boundary guards (T48kw-T50vb) - #49
Merged
Merged
Conversation
added 5 commits
September 29, 2026 16:15
…T48kw-T50vb opened PR#47 review verdict: log scale must compose with normal distributions (ln-space mu/sigma, lognormal raw draws) instead of being rejected, and four scale-boundary guards shipped weaker than their docstrings claim. Spec: V46rn, V47st, B19ps, tasks T48kw/T49dr/T50vb.
A log-scale normal now keeps its mean/std in ln space: raw draws are exp(N(mu,sigma)) (lognormal, positive by construction) and the surrogate sees exactly N(mu,sigma) in its training space -- the same contract the log-uniform branch already honors. Reaching create_manual_uq_samples, the Sobol ppf map (lognorm(s=sigma, scale=e^mu)) and the session boundary; log+constant stays rejected. Tests: 3 rejections flipped to behavior tests (Jensen direction on UQ mean, Sobol decomposition shift, Pearson move), engine-level lognormal draw test, flip matrix extended with a log-normal column over all 3 distributions-taking entry points. V&V I4/I7/I8/I9 + TIER plan wording amended. Closes T48kw (V46rn, V45ls, B19ps).
Four fixes from the PR#47 review, all confirmed against the code: - compute_correlation_indices now requires an input_scales entry per correlated variable (membership ValueError instead of the silent per-variable linear default that contradicted its docstring) - compute_correlations / generate_lhs_samples / generate_grid_samples reject preprocessing overrides for columns not in play, matching the session path's exact-cover rule - _uq_engine_distributions validates the log-uniform upper bound (present and above the lower bound) so the failure is a SumoInputError at the boundary, not a SumoEngineError surfacing from the engine - optimize Pareto docstring corrected: a log objective fits in ln space but the front is reported exp-restored in original units Closes T49dr (V47st, V45ls, V23er, B19ps).
DomainSpec and DistributionSpec now refuse maximum <= minimum in __post_init__ with a SumoInputError, instead of letting an inverted box surface as a silently descending log grid axis or a mid-engine MOGA failure. One-sided DistributionSpec bounds stay constructible; shape- specific requirements remain where the shape is interpreted. Adds TestSpecValidation (contract tests) and V&V Category I row I10 covering the full V47st guard set. Closes T50vb (V47st, V23er).
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
1 participant
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.
Why
The PR #47 review verdict (see that thread): the shipped "deliberate semantics" — log + normal input → rejected ("source parity; no invented log-normal") — was the wrong call. The log knob must compose with a normal: for a log-scale column, μ/σ describe the ln-space distribution, so raw draws are lognormal (positive by construction) and the surrogate sees exactly
N(μ,σ)in the space it trains on. That is the same contract the log-uniform branch already honors ("the distribution describes what the model sees"). The same review confirmed four scale-boundary guards that shipped weaker than their docstrings claim. Spec:V46rn/V47st, bugB19ps.What
scale="log"normalaccepted: drawsexp(N(μ,σ))— lognormal in original units (resolve_log_scale+create_manual_uq_samples);constantstays rejectedlognorm(s=σ, scale=e^μ)ppf — decomposition over what the model sees_uq_engine_distributionsboundarydistributions-taking entry points must move when a normal column turns log (V46rn machine guard)compute_correlation_indicesinput_scales[var]is a required key — a variable missing from the map raises instead of silently defaulting to linear (docstring stops overstating)compute_correlations/generate_lhs_samples/generate_grid_samplespreprocessingoverrides for columns not in play — the session path's exact-cover rule, everywhere_uq_engine_distributionsSumoInputError, not a mid-engineSumoEngineErrorDomainSpec/DistributionSpecmaximum ≤ minimumrefused at construction (__post_init__) — kills the silently-descending inverted log grid axis; MOGA inverted domains become input errorsoptimizedocstringCopilot review items #1–#4 from #47 are all addressed here. V&V Category I amended (I4/I7/I8/I9) + new I10 row for the guard set; TIER plan wording aligned.
Verification
TestSpecValidationcontract tests; flip matrix extended.ruff check/ruff format --check/ty checkclean (prek-equivalent).