Repository navigation
feat(api): honour log-scale across every workflow (T27fr) - #47
Conversation
Port the mmux_vite fullstack-logscale backend transforms into the library. DataPreprocessor (V21pf/V22rs): - add log_transform to VariableConfig + setup_log_transform/_configure_log_transform - natural-log applied in _fit/_transform_variable_group, exp restored in inverse_transform (both input and output groups) - inverse_transform_output_std: delta-method std_orig ~= |y_hat_orig| * std_log, with normalization-aware scaling; falls back unchanged + warns without points api._session: - honour PreprocessingSpec scale: drop the "log not supported yet" rejection, drive setup_log_transform from the effective per-column scale in fit() - route predicted std through the delta-method inverse in cross_validate and along_axes (linear path is a plain remap, unchanged) - reject non-positive log columns and non-positive held log values as SumoInputError before Dakota runs (V23er taxonomy, not the raw ValueError) Tests: preprocessor round-trip / mixed-scale / positivity / delta-method (unit) + real-Dakota cross_validate/along_axes/grid original-space units and the rejection guards (integration); replace the obsolete "unsupported scale" api-contract test with honour + reject cases. 332 passed, ruff/ty clean. T27fr stays ~: UQ/MOGA log paths inherit via the preprocessor but are untested here, and the domain/distribution config split is the other half.
Log now reaches the uncertainty sampler and accuracy metrics, not just the surrogate fit. - data.create_manual_uq_samples: add the log_scale branch — a log-scale uniform is drawn uniformly in log10 space and returned in the caller's original units (log-uniform); log_scale with a non-uniform distribution or a non-positive bound raises. Faithful to the mmux_vite source. - api._session._uq_engine_distributions: flag log-scale variables as log_scale for the sampler, and reject log-scale + non-uniform / non-positive-lower-bound at the boundary as SumoInputError (V23er) instead of the sampler's raw ValueError. - evaluate_cv_metrics already composes cross_validate (now log-aware), so a log-scale response flows to the metrics through the preprocessor. Tests: create_manual_uq_samples log-uniform / reject cases (unit) + real-Dakota evaluate_uncertainty log input skews response low, log+normal and log+min<=0 rejections, and cv_metrics honouring / rejecting log (integration).
Complete "log everywhere" for the optimizer. - api._session.optimize_pareto_front: accept a PreprocessingSpec; a log-scale variable is trained and explored in log space (its search domain is mapped into ln space and must be strictly positive -> else SumoInputError), a log-scale objective is fitted on ln(y) with the Pareto front exp-restored to original units via the preprocessor inverse. - api.optimize: forward the preprocessing spec. Tests: real-Dakota MOGA with a log objective returns an original-units front, and a log variable with a non-positive domain bound is rejected.
Complete "log everywhere" for the sensitivity workflow. - api._session.sobol: route distributions through _uq_engine_distributions, so a log-scale variable is flagged log_scale and rejected (SumoInputError) unless it carries a strictly-positive uniform -- same contract as uncertainty. - evaluate.evaluate_sobol_indices: build the Saltelli ppf in log space for a log-scale variable (scipy.stats.loguniform -> log-uniform in the caller's original units); the surrogate's preprocessor re-applies the log downstream. - Reword the shared guard as "distribution" not "uncertainty" so it reads right for both UQ and Sobol. Test: real-Dakota Sobol with a log input shifts the variance decomposition in the expected direction (compressed width explains less, height's share rises), plus log+normal and log+min<=0 rejections.
Record the "log everywhere" behaviour just shipped in code. - V44ls: a scale="log" override must be honoured by every scale-consuming api workflow (surrogate fit, UQ/Sobol sampling, MOGA search domain, CV metric), never silently ignored outside the surrogate fit. - B17sc (backprop): log reached the surrogate but was a near-no-op for UQ/MOGA/cv-metrics/Sobol after the initial port; the fix wires log-space into each path, guarded by V44ls. - T27fr note: backend log-scale now complete end-to-end; remaining scope narrowed to the domain⊥distribution split (V26dd) + mmux_vite frontend consumer migration.
version-check (V28tz) requires a develop PR's version to be strictly newer than every existing tag; v0.1.0a5 is already tagged at develop's tip, so this PR must carry a6. pyproject + uv.lock self-version moved together (uv lock --locked clean).
scale must reach every value-producing entry point (samplers + correlations included), enforced structurally (required-arg helpers) + behaviourally (linear<->log flip-test). Decisions locked in review discussion.
Pre-existing ty diagnostic (unguarded .group on re.search) reding prek on every PR into develop, incl. #47. Walrus + assert keeps the test strict (silently skipping a block would hide config drift).
V45ls: every value-producing api entry point takes scale and applies it. - data.funs_data_processing: one unit->value map scale_distribution(min,max,*,scale) (uniform vs scipy.loguniform) + resolve_log_scale + scale_values; scale is a REQUIRED arg -- unwired value producers break at call with TypeError, never silently default. Absorbs the log10-power hand-roll and the duplicated log guards in manual-UQ sampling and the Sobol ppf path. - api.generate_lhs_samples / generate_grid_samples: accept preprocessing; log domains fill log-uniform/geometric (linear default bit-identical); non-positive log domain -> SumoInputError. - api.compute_correlations: accepts preprocessing; compute_correlation_indices requires input_scales/output_scale (Pearson moves under log, Spearman is monotone-invariant, asserted). - api._session: _is_log closure + method getter collapse into module-level column_scale(spec, column). Tests: correlator unit tests adopt required scales + new scale-shift/invariance and TypeError-tripwire cases. 359-pass suite, ruff/ty clean.
- TestScaleFlipMatrix: all 10 public value-producing entry points' outputs must move when width turns log -- the machine guard against silent scale-ignore, shipped or future. - TestScaleAwareSamplers: log LHS/grid fill log-uniform, linear default unchanged, non-positive log domains refused, correlations honour log (Pearson moves, Spearman bit-identical, untouched column identical). - TestScaleGapCoverage: MOGA log objective + maximize (sign-after-log inverse order), log-variable sweep geometric in original units, log-response uncertainty multiplicative spread, Sobol mixed log+constant partition. - Existing grid test: cast result.data for repo-wide ty (fixes the prek red on #47).
verification-validation.md: new Category I (I1-I8, incl. H6 preprocessor log round-trip), status counts 245->359 / 26->30, engine refs 1.5.9/6.20 -> 6.24.7, summary A-H -> A-I. TIER plan: section 7 documents the scale helper contracts (required-scale tripwires, log guards, delta-method std inverse). SPEC: T46ls x.
…(T47mt) The 11th value-producing entry point. The web UI's correlation workflow (/compute_correlation_indices, #470) draws Monte Carlo samples from caller distributions, predicts them through the fitted surrogate, and correlates each input against the prediction over the SHARED sample set. Until now the api only offered table-mode compute_correlations, so a consumer migration would either keep the computation in flaskapi or silently degrade to correlating the training table (what superseded branch #537 did unnoticed). - evaluate/funs_evaluate: correlate_manual_uq_samples -- the endpoint's sampling chain, with input_scales/output_scale REQUIRED (V45ls tripwire; no erfinv noise -- Pearson/Spearman run on the surrogate's own prediction). - SumoSession.correlations: exact-cover guard + log flags via the existing _uq_engine_distributions (log requires uniform with min > 0). - api.evaluate_correlations: one-shot workflow mirroring evaluate_uncertainty. - CorrelationResult gains seed (default None; table mode unchanged). Tests: dominant-variable recovery, seed reproducibility, log-mode movement, guard rejections, TypeError tripwire, __all__ surface, flip matrix now over all 11 entry points. Suite 365 passed; ruff/ty clean.
B18mt records why the surface had a hole (T25dp landed only table-mode correlation) so no future consumer migration re-degrades the endpoint. V&V report: flip-matrix row says 11, new I9 for evaluate_correlations, counts 359->365. TIER plan documents the new producer's contracts.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Scale maps and bounds still have paths that are silently ignored or surface the wrong error type.
Review effort: Balanced
Findings: 1
Open (4)
What changed in this PR
Extends log-scale handling across surrogate fitting, sampling, optimization, Sobol analysis, and correlations.
Changes:
- Adds shared scale transformations and validation.
- Introduces surrogate-based
evaluate_correlations. - Expands scale-focused tests and documentation.
| File | Description |
|---|---|
uv.lock |
Updates package version. |
pyproject.toml |
Bumps version to 0.1.0a6. |
SPEC.md |
Records scale requirements and tasks. |
docs/verification-validation.md |
Adds log-scale V&V coverage. |
docs/TIER1_TIER2_UNIT_TESTS_PLAN.md |
Documents scale helper contracts. |
src/itis_sumo/api/__init__.py |
Exports correlation evaluation. |
src/itis_sumo/api/_session.py |
Threads scale through session workflows. |
src/itis_sumo/api/types.py |
Adds correlation seed metadata. |
src/itis_sumo/api/workflows.py |
Adds scale-aware public workflows and samplers. |
src/itis_sumo/data/funs_data_processing.py |
Adds shared scaling and scaled correlations. |
src/itis_sumo/evaluate/funs_evaluate.py |
Adds surrogate correlations and log-aware Sobol sampling. |
src/itis_sumo/preprocess/data_preprocessor.py |
Implements log transformation and uncertainty inversion. |
tests/test_api_contract.py |
Updates public API contracts. |
tests/test_api_workflows.py |
Covers scale behavior across workflows. |
tests/test_correlation_indices.py |
Tests scaled correlation computation. |
tests/test_dakota_funs_data_processing.py |
Tests log-uniform UQ sampling. |
tests/test_data_preprocessor.py |
Tests log preprocessing round trips. |
tests/test_dependabot_config.py |
Fixes type narrowing in configuration tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| input_array = scale_values( | ||
| input_samples[var], scale=input_scales.get(var, "linear") | ||
| ) |
| if spec.minimum is None or spec.minimum <= 0: | ||
| raise SumoInputError( | ||
| f"'{variable}' is log-scale but its distribution lower " | ||
| "bound is not strictly positive" | ||
| ) |
| missing = sorted((set(variables) | {response}) - set(samples.columns)) | ||
| if missing: | ||
| raise SumoInputError(f"Samples do not contain columns: {missing}") | ||
| spec = preprocessing or PreprocessingSpec() |
| A ``scale="log"`` override on a variable explores it in log space; on an | ||
| objective it fits and reports the front in log space. |
|
@alexpargon could you launch Q against this PR? it is usually good at spotting mishaps ;) |
alexpargon
left a comment
There was a problem hiding this comment.
Review — the tripwires are real, the std inverse is a genuine bug fix, and the flip matrix has teeth
Reviewed the full diff plus the head-sha sources for every claim worth checking (_session.py, workflows.py, funs_data_processing.py, funs_evaluate.py, data_preprocessor.py, the test files, SPEC/V&V docs) and the CI run. This is what a second-phase scale rollout should look like: the guard removed from _validate_samples ("Logarithmic scale is not supported yet") is replaced by actual support and by machinery that makes silent drift structurally impossible.
Good / no action needed
- The structural tripwire is honestly enforced.
scale_distribution(minimum, maximum, *, scale),correlate_manual_uq_samples(..., input_scales=, output_scale=)andcompute_correlation_indices(..., input_scales=, output_scale=)all take scale as required keywords — and theTypeErrorbehavior is tested, not just asserted in a docstring (test_producer_requires_scales,test_scales_are_required_arguments). Oneunit→valuehome for LHS/grid/UQ draws/Sobol ppfs is the right consolidation; the base-agnosticloguniformnote is correct. TestScaleFlipMatrixcovers exactly the 11 named entry points — verified the case list one by one against the body's claim (10 table-mode workflows +evaluate_correlations) — and theassert len(cases) == 11inside the test means a future entry point can't quietly skip the matrix by omission from this list… well, it pins the count, which is the point.inverse_transform_output_stdfixes a pre-existing bug beyond log. At base,cross_validate.predicted_stdwent through_to_original_units→_denormalize_value, which for z_score doesstd * config.std + config.mean— a mean shift applied to a standard deviation, wrong even before log existed. The new multiplicative-only rules (z_score×σ, min_max×(max−min), log×|ŷ|delta method, sign-switch magnitude-preserving) are the correct math, applied to bothcross_validateandalong_axes(basealong_axespassed the log-space std through raw — the "width, not position" docstring says the bug was noticed and closed here).- The forward/inverse transform order round-trips. Forward is log → sign → normalize; the inverse in
inverse_transformis denormalize → sign-flip →exp— verified thenp.expsits after both, so a log objective under maximize (MOGA) comes back correctly. The I6 test asserts both directions, which is the case most likely to rot. - The UQ histogram path needed no delta method and the PR knows it:
propagate_manual_uq_with_uncertaintyinjects the log-space std into samples and inverse-transforms the sample array, so the spread becomes multiplicative in original units on its own — andtest_uncertainty_log_response_spread_is_multiplicativelocks that in. Checked this specifically because it's the path where a naive port would have double-applied the Jacobian. - Guard placement is defense-in-depth without being noisy: log+non-uniform and log+
min ≤ 0rejected asSumoInputErrorat the api boundary (_uq_engine_distributions), re-checked at the sampler layer (resolve_log_scale), positivity of samples/held-values/domains all refused pre-Dakota; the delta method warns and returns unchanged rather than fabricating a std when point estimates are missing.compute_correlationswraps the engine'sValueErrors intoSumoInputError, so V23er holds on the new path. evaluate_correlationsis the right home for the #470 chain. Exact-cover distribution validation, seed recorded onCorrelationResult, and it's the flow our flaskapi will migrate onto — thank you for the heads-up; SPEC B18mt's framing of the #537 silent-degradation failure mode is exactly the trap we'd have walked into.- CI green across 3.10–3.13 + prek/version/docs; the
test_dependabot_config.pytyfix is the honest kind (assert-narrowing rather than# type: ignore), and the V&V engine-reference refresh (6.20 → 6.24.7) matches the actualitis-dakota==6.24.7pin already in base — the doc was stale, this PR corrects it.uv.lock's self-version was also a rung behind (a4 vs a5); this lands it at a6 consistent with pyproject.
Meaningful
compute_correlation_indicesdocstrings "REQUIRED, never defaulted", but the loop defaults per variable:scale_values(input_samples[var], scale=input_scales.get(var, "linear"))(funs_data_processing.py:805). The mapping is required, so the session path (which always builds full mappings after exact-cover validation) is airtight — but a direct engine-layer caller who omits one variable frominput_scalesgets a silent linear default for exactly that column, which is the failure mode V45ls exists to prevent. One-line fix: index withinput_scales[var](or membership-check and raise next to the existing "not found in input samples" check). Small blast radius, hence not blocking — but the docstring currently overstates the guard.
Minor (optional)
- PR body says "new Category I (I1–I8 + H6)"; the doc actually lands I1–I9 (I9 is the
evaluate_correlationsrow). Body undercounts its own best row. - Neither
DomainSpecnorDistributionSpecvalidatesminimum < maximum. Nothing log-scale slips through silently (scale_distribution's log branch rejectshi <= loat draw time, and MOGA's inverted ln-domain dies loudly in the engine), so this is a pre-existing ergonomics gap, not a hole opened here — a pydantic-style ordering check on the specs would turn the MOGASumoEngineErrorinto aSumoInputErrorsomeday.
Verdict: Approving. The input_scales[var] tightening is worth its own small follow-up; nothing here should hold the merge.



Why
scale="log"on a column reached the surrogate fit but was a near-no-op for UQ manual sampling, MOGA, cv-accuracy-metrics, Sobol, the standalone samplers and correlations. The decision driving this PR: log applied everywhere — and made un-forgettable, so no current or future value-producing path can ignore it.What — two phases
Phase 1 (T27fr, V44ls/B17sc): log honoured in every surrogate workflow.
scale="log"columnln(x), exp-restore results; delta-method std inversecreate_manual_uq_samplesdraws log-uniform; log + non-uniform or non-positive bound →SumoInputErrorcross_validateln(y), front exp-restoredPhase 2 (T46ls, V45ls/V21pf-amendment): the whole surface + dedup.
generate_lhs_samples,generate_grid_samples,compute_correlationsnow takepreprocessing(default linear — every existing call unchanged) and honour scale: log domains fill log-uniform/geometric; correlations run on scaled values (Pearson moves, Spearman provably doesn't).unit→valuemapscale_distribution(min, max, *, scale)+resolve_log_scale+ the correlator'sinput_scales/output_scaletake scale as required args — code producing values without threading scale breaks at call withTypeError, never silently defaults. Absorbs the earlierlog10-power hand-roll, duplicate log guards, and the optimizer's_is_logclosure.TestScaleFlipMatrixasserts all 11 public value-producing entry points' outputs move when a column turns log.api.evaluate_correlations(new, 11th entry point, T47mt/B18mt): the web UI's correlation workflow (#470) — draw MC samples from caller distributions → predict through the fitted surrogate → Pearson+Spearman over the SHARED sample set — moves into the api scale-native (engine producer requiresinput_scales/output_scale). Without it, the flaskapi migration could only keep the computation in flaskapi or silently degrade to table correlation (what superseded branch #537 did unnoticed).docs/verification-validation.md: new Category I (I1–I9 + H6) folds the log-scale coverage into the V&V report; counts refreshed; stale engine refs fixed. TIER plan documents the helper contracts.tyred intest_dependabot_config.py(unrelated, was blockingprekon every PR) fixed in its own commit.Deliberate semantics
ln(y)).PreprocessingSpec(domain boxes stay scale-free, V26dd).Verification
ruff check/format+ repo-widety check(V41hp) clean.Spec
Notes
0.1.0a6(develop tip already taggedv0.1.0a5; V28tz strictly-newer gate).uv lock --lockedclean.🤖 Generated with opencode