Repository navigation
[AI-Assisted] fix(characterization): preserve shared quantile cut placement - #4330
Merged
Merged
Conversation
Retain out-of-gap quantile placement through representable open-gap clamping. Add public-API redistribution and edge regressions, clarify constrained mass semantics, and regenerate engineering coverage. Fixes #4307.
This was referenced Oct 14, 2026
Extract the existing general-flash ownership fix as a small foundation for parallel compressor and process evaluation. Preserve equations and tolerances. AI-assisted implementation and review.
EvenSol
changed the base branch from
master
to
fix/shared-classic-mixing-accumulators
October 14, 2026 19:57
Base automatically changed from
fix/shared-classic-mixing-accumulators
to
master
October 15, 2026 03:55
EvenSol
marked this pull request as ready for review
October 15, 2026 03:56
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.
Current-master coverage reconciliation — 2026-10-15
Integrated master
3566670and regenerated coverage after its platform-modeling skill update added GOR/Recombine mappings. This repairs the shared hosted stale-inventory failure; no Java source, numerical equation or tolerance changes accompany this reconciliation. Exact head:aa6e0e6822c024ed287c6d44a4740e3679bbe1bf; verified local/remote tree:7bde329047ca07f913f50624bd0f0e38ee84bbba.All 17 engineering-coverage tests, generated inventory/check, direct Spotless check, documentation-search audit, both all-files hook stages and whitespace checks pass. Existing numerical test evidence applies to the unchanged implementation; hosted CI is rerunning. Documentation impact: retain master's skill guidance and refresh its generated mappings. Existing qualification/review gates remain.
Master integration — 2026-10-15
Integrated master
1f6d1c1inedfad9308bee9345b2a8ea23c68c0bb02f5edf7e. Verified local/remote tree:e9a51421b490f87db3cdf7de193b03919df8804f. Preserve both task histories, retain the already-merged shared mixing correction, and regenerate engineering coverage. No characterization equation, tolerance, or production behavior changed in this integration.All 40 focused characterization tests pass using
pomJava8.xmlon OpenJDK 17, with zero failures/errors/skips. Direct Spotless apply/check, documentation-search audit, generated coverage check, both all-files hook stages and whitespace checks pass. Documentation impact: retain the existing mathematical/usage guides and both task histories. Fresh hosted CI and applicable independent review remain required.Coordinated stack — 2026-10-14
Published head:
ef3bf468e2a9fdf21d0206cf5b4aa81a6eeebeff; verified tree:b4cb1f89e5954641911c960ad2ae7927ceb922ea.Coordination and validation update
This PR is stacked on #4333 (
fix/shared-classic-mixing-accumulators). Its original characterization implementation, regression test and two guides are preserved byte-for-byte. The stack carries the published foundation and master workflow updates, merges the task log and regenerates engineering coverage for the combined tree.The shared dependency is verified: the unchanged ten-well field-to-facility fixture enables optimized parallel/dataflow execution, and its HP/LP separator, scrubber and compressor branches share the same two phase-specific EOS handlers. The foundation isolates the three covered calculation accumulators while preserving that sharing.
The complete unchanged
FieldToFacilityOptimizationAcceptanceTestpassed three fresh-JVM runs on both original #4330 and the foundation overlay: six tests per variant, zero failures or skips. The test's default execution settings, constraints and1e-9replay tolerance were unchanged. Class-load checks confirmed the intended handler overlay. Validation used Linux OpenJDK 17, Java 8 POM dependencies and release-8 bytecode; each JVM was capped at 1 GiB.These results establish the shared-handler exposure and local integration compatibility. They do not prove the cause of the earlier Windows Java 8
ACCEPTED_POINT_REPLAY_FAILEDfailure: the unchanged original head also passed locally, and the hosted failure supplied no numerical replay residual or failing constraint. Windows Java 8 CI remains a gate; no tolerance relaxation is proposed.Direct Spotless apply/check, both pre-commit hook stages, documentation search, engineering-coverage generation/check and documentation/coverage contract tests passed on the exact stacked tree. Formatting preserved all Java source bytes. Merge remains contingent on successful checks for the published stack, #4333 landing first and the applicable accountable characterization/thermodynamic review. Agent validation does not substitute for maintainer approval.
Problem and fix
sharedImaginaryBoundariesdiscarded finite mass-quantile candidates outside their corresponding reference gap and silently reused the midpoint. This made the option ineffective for many cuts.Project each finite candidate into
[Math.nextUp(lower), Math.nextDown(upper)], retaining candidates already inside the gap. Keep the legacy fallback for unavailable/non-finite candidates, degenerate gaps, and resolution <= 1. The default flag-off path and the delumping/quantile algorithms are unchanged. The correction adds only constant work per reference cut in the opt-in path; no flash-loop changes or measured speedup claims.Closes #4307.
Review evidence
On the issue's six-cut reference at resolution 12, the midpoint boundaries are 360.579, 403.8965, 451.7015, 501.09 and 550.83 K. Corrected boundaries are approximately 359.860359, 429.023, 474.38, 527.8 and 573.86 K: all five differ. The last four are the representable values immediately below their upper reference keys.
The original three coarse source lumps still occupy the same bins: no source key crosses the changed boundaries. Identical output from that sparse example alone does not establish a no-op. Public-API regressions therefore use explicit source probes on both sides of the affected cuts, and a delumped version of the reported source verifies an observable composition change.
Two new clamp-direction regressions fail on unchanged master (450 K assigned to R2 instead of R1; 350 K assigned to R1 instead of R2) and pass after the fix. Coverage also checks interior candidates, exact boundary inclusion, equal/adjacent-double gaps, reference identity, flag-off compatibility, and mole/mass conservation with and without source delumping.
Documentation impact
Updated accessor/builder and implementation Javadocs, the characterization mathematics guide, and the combining guide. These are constrained mass-quantile cuts; neither projection nor a discrete grid guarantees equal final cut masses. Documented fallback behavior and the distinction between conserving source-derived properties and overwriting molar masses through inheritance. Regenerated the two affected source digests in engineering coverage.
Validation
Base:
b0de5165abe8582dbfef1ea1b78fc87c11e6e043. Published commit:6226b1fed968aab1caf4e61d359e4024fd2cde0f. Source/build inputs verified against current master. NeqSim 3.25.0, Java 17 under the current POM; added Java APIs/syntax are Java 8 compatible, but no Java 8 runtime test is claimed.All commands below exited 0 unless otherwise described:
./mvnw -B -ntp -Dtest=PseudoComponentCombinerTest,CharacterizationOptionsTest,CharacterizeToCommonSlateTest,CharacterizeToReferenceCommonSlateTest,CharacterizeToReferenceDelumpTest,CharacterizeToReferenceSharedSlateTest test: 40 passed, 0 failures/errors/skips (11 shared-boundary tests).python3 devtools/run_spotless.py apply: passed, repeated until stable.pre-commit run --all-files --hook-stage pre-commit: passed.python3 devtools/run_spotless.py check: passed.python3 devtools/check_documentation_search.py: passed (788 Markdown pages plus one HTML page).pre-commit run --all-files --hook-stage pre-push: passed.python3 devtools/build_engineering_coverage.pyand--check: passed../mvnw -B -ntp checkstyle:check: passed, 0 enforced violations; existing warning-level findings remain.java -m jdk.javadoc/jdk.javadoc.internal.tool.Main -quiet -Xdoclint:all -privateon both modified production classes: passed; existing missing-comment warnings remain.git diff --check: passed.Maven used the NeqSim skill's environment-only bootstrap. This is an AI-assisted numerical bug fix restoring the documented projection intent, not independent qualification of the synthetic delumping distribution.
Earlier implementation validation is retained above as historical evidence. The current stacked-head gates and remaining Windows replay qualification are recorded at the top.