Skip to content

Unit - #3801

Open
asmfstatoil wants to merge 32 commits into
masterfrom
unit
Open

Unit#3801
asmfstatoil wants to merge 32 commits into
masterfrom
unit

Conversation

@asmfstatoil

@asmfstatoil asmfstatoil commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Pull Request

Thank you for contributing to NeqSim! Please complete the following checklist before requesting review:

  • Confirm ./mvnw test runs successfully
  • Verify code formatting using Checkstyle
  • Update any relevant docs/README
  • Link to an associated issue or discussion
  • Request the applicable CODEOWNERS and record independent domain review where required
  • Add an accepted NRC and migration note if this changes a major public contract

See CONTRIBUTING.md, GOVERNANCE.md, and
the API lifecycle policy for the full contribution process.

CI repair — 2026-09-18

Repair f29e3cd058943314041186368b3f4e466b83f5d6 fixes the missing PressureUnit conversion API call in Filter and the subsequently reproduced RateUnit stack overflow. Filter pressure drops now use the new conversion API without gauge offsets; rate conversion obtains mol/s directly from the stored value and preserves instance-specific fluid properties for explicit conversions.

Added ten differential-pressure unit cases (including zero/negative inputs) and four rate-conversion regressions. All unit-conversion, filter and mixer suites passed with ./mvnw -q '-Dtest=neqsim.util.unit.*Test,FilterTest,MixerTest,StaticMixerTest' test: 70 tests, zero failures/errors, one existing skip. The safety-documentation regression also passed both tests.

Documentation impact: clarified differential-pressure semantics and rate-conversion behavior in Javadocs and guides.

Current master introduced a new Spotless failure in SafetyRoadmapDocumentationTest.java during this repair. Carried that current test and its matching safety roadmap into this branch, corrected the array-brace spacing, and removed the roadmap's trailing blank line so the branch can validate the fixture independently. These two files reproduce current master content apart from those whitespace fixes.

Final validation: direct python3 devtools/run_spotless.py apply (clean repeat), python3 devtools/run_spotless.py check, python3 devtools/check_documentation_search.py, pre-commit run --all-files --hook-stage pre-commit, pre-commit run --all-files --hook-stage pre-push, whitespace checks, and ./mvnw -q -Dtest=SafetyRoadmapDocumentationTest test all passed (exit 0; two safety-doc tests). Maven used the NeqSim bootstrap helper. Final repair: f29e3cd058943314041186368b3f4e466b83f5d6. VALIDATION PENDING CI; the full Java/platform matrix is not claimed complete.

Final conflict resolution — 2026-09-18

Final head: 7b2c48c86ade36eec6135249562e6a454ecfb153. The initial backport conflicted with master's independently added safety test. This follow-up explicitly joins exact master 1cd176898896a30ddb5a6b6c5a3abc7c771feea3 to the existing repaired PR head, resolving only the safety roadmap and test conflicts in favor of the formatted copies. No history was rewritten and no PR was merged into master. This supersedes the earlier no-master-merge description of the formatting-only stage.

Git's merge result, with only those two conflicts resolved, exactly matches the previously validated tree 7bd1fb813019c875aa503d03a2355f9266422373. Re-ran direct Spotless apply/check, documentation-search audit, both pre-commit stages and whitespace checks on that final tree; all passed (exit 0). Reused the focused test evidence above because the conflict-resolved tree is byte-identical to it. VALIDATION PENDING CI on this final merge head.

CI repairs (2026-09-20)

Three independent failures on the original head are addressed:

  • Unsupported units now raise IllegalArgumentException while retaining the detailed InvalidInputException cause. This restores the distillation endpoint temperature contract; constructor, instance, and static conversion checks cover it.
  • Binary feasibility search records the lower endpoint before searching. If interior candidates fail the final replay, the optimizer can recover to a freshly verified lower endpoint. A deterministic regression failed before the fix and passes after it; no feasibility checks were relaxed.
  • The Python optimization tutorial uses three-point differences instead of a coarse eps=0.1 forward difference, eliminating the L-BFGS-B false descent/line-search failure. It asserts convergence, re-evaluates the selected point, and checks agreement with Powell. Executed outputs, figures, generated Markdown, and the documentation link are synchronized.

Validation on OpenJDK 17:

  • Targeted distillation, bottleneck, optimizer, and temperature suites: 29 tests, zero failures/errors, one existing skip.
  • Complete unit-conversion package: 44 tests, zero failures/errors, one existing skip (overlaps the targeted suite).
  • Documentation tests: 385 passed.
  • Notebook: all 24 code cells executed successfully in a fresh IPython subprocess with SciPy 1.18.1; 3 plot outputs. Reviewed all 44 cells, plots, and tables through nbconvert HTML / WeasyPrint / Poppler; no LaTeX equations are present. The final link-only correction was re-executed and produced identical outputs.
  • Direct Spotless apply twice, Spotless check, documentation search audit, pre-commit and pre-push hooks, and whitespace checks passed.

Documentation impact: updated unit error semantics, feasibility bracket/replay guidance, and the executable tutorial plus generated page/figures. Java 8 and the complete CI matrix remain VALIDATION PENDING on the updated head.

Remove BaseUnit's getValue stub, which shadows LinearScaleUnit's default, and implement LinearScaleUnit for fluid-dependent RateUnit. Restore a dispatch regression through Unit and LinearScaleUnit references, checking independent expected values, stored-state preservation and unsupported targets.

Also bracket feasible compressor flow before binary upper-bound optimization, enforcing a strict 100% utilization limit; document this precondition and apply the release documentation fix from #3836.
Validation: reproduced 10 conversion errors before the fix. Afterward 136 affected tests complete with zero failures/errors and one existing skip, plus all 10 bottleneck tests. 385 documentation tests, direct Spotless apply/check, search audit and both hook stages pass. JDK 17; full CI pending.

EvenSol commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

Fixed in 6a81f69. BaseUnit.getValue() was shadowing the interface default, and RateUnit no longer implemented a conversion strategy. Removing the stub and implementing LinearScaleUnit restores scalar and fluid-dependent conversions. A regression checks calls through both interfaces, independently expected values, unchanged stored state, and invalid targets.

Also corrected the existing two-stage compressor test to find a feasible bracket before binary search; surge/minimum-speed limits make the full range non-monotonic. Its utilization assertion is tightened to 100%. Applied the five-page release documentation correction from #3836.

Validation: before the fix, 17 selected unit tests had 10 errors. Afterward, 136 affected tests completed with zero failures/errors and one existing skip; all 10 bottleneck tests also pass. All 385 documentation tests, direct Spotless apply/check, documentation audit, and pre-commit/pre-push hooks pass. The unit guide and optimization guidance are updated. Local JDK 17; fresh full CI, including notebook jobs, is pending.

EvenSol commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

Additional local verification: both optimization notebooks that failed in the previous CI run now execute successfully against the fixed build: NeqSim_Python_Optimization and norwegian_ncs_oil_network_optimization. Verified with the repository's devtools/check_optimization_notebooks.py --executor ipython in a fresh output directory (both PASS). The IPython executor was used because this local environment blocks the notebook kernel's ZeroMQ sockets.

@asmfstatoil
asmfstatoil marked this pull request as ready for review September 20, 2026 14:14
EvenSol and others added 2 commits September 20, 2026 17:05
Preserve a verified lower-bound fallback in binary feasibility search, use the documented unsupported-unit exception contract, and execute the Python optimization tutorial with accurate three-point finite differences. Refresh notebook outputs, figures, and generated documentation.
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.

2 participants