Adaptive relaxation, diagnostics and safety fixes for two-phase TDMA solver; trim stream refs; add tests/docs by EvenSol · Pull Request #63 · EvenSol/neqsim

EvenSol · 2026-04-08T19:07:39Z

Motivation

Improve robustness and convergence of the two-phase TDMA solver by adding adaptive relaxation and richer convergence diagnostics.
Prevent numerical instabilities from tiny denominators and non-finite updates in phase/velocity/temperature/composition updates.
Harden reference resolution for process models by trimming whitespace to avoid brittle stream lookups, and document speed/stability evaluation.

Description

Added a new developer document docs/development/TWO_PHASE_FLOW_SPEED_STABILITY_EVALUATION.md describing solver architecture, stability guards, performance characterization and recommendations.
Fixed a fragile residual check in TwoPhaseFlowNode.initFlowCalc by protecting the denominator with Math.max(Math.abs(f), 1e-20) and corrected the iteration-limit check from 10000 to 100.
Extended TwoPhaseFixedStaggeredGridSolver with solver safety and diagnostics: introduced RESIDUAL_EPSILON, relaxation bounds, SolverDiagnostics class, calcRelativeResidual and updateAdaptiveRelaxation helpers, and getLastSolverDiagnostics() accessor.
Made initialization methods accept caller-provided relaxation factors (initPressure, initVelocity, initTemperature, initPhaseFraction, initComposition) while preserving backward-compatible no-arg defaults that use the legacy relaxation.
Replaced raw relative-residual computations with the safe calcRelativeResidual, and integrated adaptive relaxation into momentum, phase-fraction, energy and composition solve loops with per-loop iteration caps and diagnostic flags when limits are hit; recorded iteration counts and termination reason in lastSolverDiagnostics and emitted logging when diagnostics are enabled.
Added mass-transfer summary and minor mass-transfer/diagnostic helpers to aid validation.
Trimmed and normalized input in ProcessSystem.resolveStreamReference to tolerate surrounding whitespace and inconsistent casing in port tokens.
Added JsonProcessBuilderTest#testBuildWithWhitespaceAroundStreamReference to verify whitespace-tolerant stream reference resolution.

Testing

Executed targeted solver tests: TwoPhaseFlowNodeTest (real ~1m16s), TwoFluidPipeIntegrationTest (real ~0m42s) and TwoFluidPipeBenchmarkTest (real ~0m54s); these test runs completed successfully and demonstrate acceptable runtime for CI-scale usage.
Added unit test JsonProcessBuilderTest#testBuildWithWhitespaceAroundStreamReference to cover the resolveStreamReference change and included it in the test suite; the new test was run locally as part of the process-model tests and passed.
No automated test failures were observed for the modified code during the targeted runs described above.

Codex Task

Copilot

Pull request overview

This PR improves robustness and observability of the two-phase TDMA pipe flow solver (adaptive relaxation, safer residual handling, richer diagnostics) and hardens process-model stream reference resolution by trimming whitespace, with accompanying tests and a new developer evaluation document.

Changes:

Added adaptive relaxation + diagnostics scaffolding to TwoPhaseFixedStaggeredGridSolver, and added epsilon-protected residual logic in TwoPhaseFlowNode.
Made ProcessSystem.resolveStreamReference(...) tolerant of surrounding whitespace (and normalized port tokens).
Added a regression test for whitespace-tolerant stream references and introduced a new developer doc describing solver speed/stability.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
src/test/java/neqsim/process/processmodel/JsonProcessBuilderTest.java	Adds a test covering whitespace around stream references.
src/main/java/neqsim/process/processmodel/ProcessSystem.java	Trims stream reference tokens to make resolution less brittle.
src/main/java/neqsim/fluidmechanics/flowsolver/twophaseflowsolver/twophasepipeflowsolver/TwoPhaseFixedStaggeredGridSolver.java	Introduces adaptive relaxation, safer residual calculation helpers, and solver diagnostics tracking/accessors.
src/main/java/neqsim/fluidmechanics/flownode/twophasenode/TwoPhaseFlowNode.java	Protects a residual denominator against tiny values and corrects the iteration-limit check.
docs/development/TWO_PHASE_FLOW_SPEED_STABILITY_EVALUATION.md	Adds developer-facing evaluation notes on algorithm architecture, stability guards, and runtime.

Copilot · 2026-04-08T19:14:52Z

src/main/java/neqsim/process/processmodel/ProcessSystem.java

+    if (normalizedRef.contains(".")) {
+      String[] parts = normalizedRef.split("\\.", 2);
+      unitName = parts[0].trim();
+      port = parts[1].trim().toLowerCase();
    } else {
-      unitName = ref;
+      unitName = normalizedRef;


resolveStreamReference lowercases the port token using toLowerCase() without an explicit locale, which can behave unexpectedly under certain default locales (e.g., Turkish). Use toLowerCase(Locale.ROOT) (and import java.util.Locale) to ensure stable, locale-independent parsing of port names.

Copilot · 2026-04-08T19:14:52Z

src/test/java/neqsim/process/processmodel/JsonProcessBuilderTest.java

+  @Test
+  void testBuildWithWhitespaceAroundStreamReference() {
+    SystemSrkEos fluid = new SystemSrkEos(298.15, 50.0);
+    fluid.addComponent("methane", 0.85);
+    fluid.addComponent("ethane", 0.10);
+    fluid.addComponent("propane", 0.05);
+    fluid.setMixingRule("classic");
+
+    Stream feed = new Stream("feed", fluid);
+    neqsim.process.equipment.separator.Separator separator =
+        new neqsim.process.equipment.separator.Separator("HP Sep", feed);
+    ProcessSystem process = new ProcessSystem();
+    process.add(feed);
+    process.add(separator);
+
+    assertNotNull(process.resolveStreamReference("  feed  "),
+        "resolveStreamReference should trim whitespace for plain stream names");
+    StreamInterface gasOutWithWhitespace = process.resolveStreamReference("  HP Sep. gasOut  ");
+    assertNotNull(gasOutWithWhitespace,
+        "resolveStreamReference should trim whitespace around unit and port tokens");
+  }


The test name testBuildWithWhitespaceAroundStreamReference is misleading: it doesn’t exercise JsonProcessBuilder.build(...) at all, it directly calls ProcessSystem.resolveStreamReference. Either rename the test to reflect what it validates, or change it to build a JSON process definition that uses whitespace in stream references and assert the build succeeds.

Copilot · 2026-04-08T19:14:53Z

src/main/java/neqsim/fluidmechanics/flownode/twophasenode/TwoPhaseFlowNode.java

+    while (Math.abs((f - fOld) / Math.max(Math.abs(f), 1e-20)) > 1e-8 && iterations < 100);

-    if (iterations == 10000) {
+    if (iterations == 100) {
      System.out.println("error in void init calc");
    }


The post-loop error condition if (iterations == 100) will print an error even if convergence happens exactly on iteration 100. Consider checking whether the loop exited due to hitting the iteration cap (e.g., iterations >= 100 and the residual criterion is still above tolerance), and prefer the project logger over System.out.println for diagnostics.

Copilot · 2026-04-08T19:14:53Z

...s/flowsolver/twophaseflowsolver/twophasepipeflowsolver/TwoPhaseFixedStaggeredGridSolver.java

+   * @return solver diagnostics
+   */
+  public SolverDiagnostics getLastSolverDiagnostics() {


lastSolverDiagnostics is transient and getLastSolverDiagnostics() returns it directly. If this solver is ever deserialized or copied via serialization, the field will be null and callers may get NPEs. Consider lazily initializing in the getter (or adding a readObject/readResolve) to guarantee a non-null diagnostics object, similar to how other transient runtime state is typically rehydrated.

Suggested change

* @return solver diagnostics

*/

public SolverDiagnostics getLastSolverDiagnostics() {

* @return solver diagnostics, never {@code null}

*/

public SolverDiagnostics getLastSolverDiagnostics() {

if (lastSolverDiagnostics == null) {

lastSolverDiagnostics = new SolverDiagnostics();

}

Copilot · 2026-04-08T19:14:53Z

...s/flowsolver/twophaseflowsolver/twophasepipeflowsolver/TwoPhaseFixedStaggeredGridSolver.java

+   * @return relative residual
+   */
+  private double calcRelativeResidual(double numeratorNorm, double referenceNorm) {


calcRelativeResidual(...) can return NaN if either norm is non-finite. In solveTDMA, NaN residuals won’t update maxDiff (since comparisons with NaN are false), which can incorrectly mark the solve as converged and stop early. Treat non-finite residuals as a hard failure (e.g., return Double.POSITIVE_INFINITY or explicitly set maxDiff to POSITIVE_INFINITY and record a termination reason) to avoid false convergence on numerical blow-ups.

Suggested change

* @return relative residual

*/

private double calcRelativeResidual(double numeratorNorm, double referenceNorm) {

* @return relative residual, or {@code Double.POSITIVE_INFINITY} if either norm is non-finite

*/

private double calcRelativeResidual(double numeratorNorm, double referenceNorm) {

if (!Double.isFinite(numeratorNorm) || !Double.isFinite(referenceNorm)) {

return Double.POSITIVE_INFINITY;

}

Copilot · 2026-04-08T19:14:54Z

...s/flowsolver/twophaseflowsolver/twophasepipeflowsolver/TwoPhaseFixedStaggeredGridSolver.java

+      double delta = relaxation * diffMatrix.get(j, 0)
+          * pipe.getNode(j).getBulkSystem().getPhase(phaseNum).getMolarMass()
+          / pipe.getNode(j).getBulkSystem().getPhase(phaseNum).getComponents()[comp]
+              .getMolarMass();
+      double updated = pipe.getNode(j).getBulkSystem().getPhase(phaseNum).getComponents()[comp].getx()
+          + delta;
+      if (updated > 1.0) {
        pipe.getNode(j).getBulkSystem().getPhase(phaseNum).getComponents()[comp].setx(1.0 - 1e-30);
-      } else if (pipe.getNode(j).getBulkSystem().getPhase(phaseNum).getComponents()[comp].getx()
-          + diffMatrix.get(j, 0) * pipe.getNode(j).getBulkSystem().getPhase(phaseNum).getMolarMass()
-              / pipe.getNode(j).getBulkSystem().getPhase(phaseNum).getComponents()[comp]
-                  .getMolarMass() < 0.0) {
+      } else if (updated < 0.0) {
        pipe.getNode(j).getBulkSystem().getPhase(phaseNum).getComponents()[comp].setx(1e-30);
      } else {
        pipe.getNode(j).getBulkSystem().getPhase(phaseNum).getComponents()[comp]
-            .setx(pipe.getNode(j).getBulkSystem().getPhase(phaseNum).getComponents()[comp].getx()
-                + diffMatrix.get(j, 0)
-                    * pipe.getNode(j).getBulkSystem().getPhase(phaseNum).getMolarMass()
-                    / pipe.getNode(j).getBulkSystem().getPhase(phaseNum).getComponents()[comp]
-                        .getMolarMass());
-        // pipe.getNode(j).getBulkSystem().getPhases()[0].getComponent(p).getx()
-        // + 0.5*diff4Matrix[p].get(j,0));
+            .setx(updated);
      }


initComposition(...) computes updated = currentX + delta and writes it without checking finiteness. If diffMatrix or molar-mass ratios produce NaN/Infinity, this will set component mole fractions to non-finite values and can cascade into thermo/flash failures. Add Double.isFinite(...) guards for delta/updated (and fall back to the current value or clamp) before calling setx(updated).

Copilot · 2026-04-08T19:14:54Z

docs/development/TWO_PHASE_FLOW_SPEED_STABILITY_EVALUATION.md

+# TwoPhaseFlow / TwoFluidPipe Algorithm Evaluation (Speed and Stability)
+
+## Scope and interpretation
+
+This evaluation targets the multiphase algorithm implemented in the `TwoPhaseFlowNode` + `TwoPhaseFixedStaggeredGridSolver` path that underpins `TwoFluidPipe` steady-state/transient multiphase behavior.
+
+> Note: the request referenced `TwoPhaseFLow`; in this codebase the relevant classes are `TwoPhaseFlowNode` and `TwoPhaseFixedStaggeredGridSolver`.
+


This new docs page is under docs/ but lacks the required Jekyll YAML front matter (---, title, description, ---) at the top of the file, which can prevent proper site indexing/search. Add the front matter block as the first lines of the document (and avoid duplicating the title immediately as an H1 if the site template renders it).

Copilot · 2026-04-08T19:14:54Z

docs/development/TWO_PHASE_FLOW_SPEED_STABILITY_EVALUATION.md

+The top-level solve uses nested loops with fixed hard caps:
+
+- Momentum loop: up to 100 iterations per phase,
+- Holdup loop: up to 100,
+- Energy loop: up to 100 per phase,
+- Composition loop: up to 10 per component with 10 outer composition passes,
+- Top-level coupled loop: up to 15 passes.
+
+Residual checks are norm-based and progressively update state through under-relaxed initialization routines (`relaxation = 0.2` in key update paths).


Sections 2.2 and 4.2 state that the solver uses fixed relaxation (relaxation = 0.2) and that TwoPhaseFlowNode.initFlowCalc uses (f - fOld) / f without epsilon protection. The PR changes introduce adaptive relaxation and add an epsilon floor in initFlowCalc, so the document is now inaccurate. Please update these bullets to reflect the current implementation and avoid documenting already-fixed risks as current gaps.

Copilot · 2026-04-08T19:14:54Z

...s/flowsolver/twophaseflowsolver/twophasepipeflowsolver/TwoPhaseFixedStaggeredGridSolver.java

+            if (!localConverged && iterLocal >= 100) {
+              lastSolverDiagnostics.momentumHitIterationLimit = true;
+              if (massTransferConfig.isEnableDiagnostics()) {
+                logger.warn(
+                    "Momentum equation reached iteration limit for phase {} with residual {}",
+                    phaseNum, diff);
+              }


massTransferConfig is transient and is dereferenced directly in solveTDMA() (e.g., massTransferConfig.isEnableDiagnostics()). If the solver instance is ever deserialized or copied via serialization, massTransferConfig may be null, leading to NPEs during solves. Consider ensuring it’s non-null before use (lazy-init in getMassTransferConfig() / at the start of solveTDMA(), or rehydrate via readObject).

Improve two-phase solver adaptivity and convergence diagnostics

5f65957

Copilot AI review requested due to automatic review settings April 8, 2026 19:07

EvenSol added the codex label Apr 8, 2026 — with ChatGPT Codex Connector

Copilot started reviewing on behalf of EvenSol April 8, 2026 19:08 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adaptive relaxation, diagnostics and safety fixes for two-phase TDMA solver; trim stream refs; add tests/docs#63

Adaptive relaxation, diagnostics and safety fixes for two-phase TDMA solver; trim stream refs; add tests/docs#63
EvenSol wants to merge 1 commit intomasterfrom
codex/evaluate-seep-and-stability-in-multiphase-flow-algorithm

EvenSol commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

EvenSol commented Apr 8, 2026

Motivation

Description

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants