-
Notifications
You must be signed in to change notification settings - Fork 10
WIP: Add NeqSim compatibility suite #1560
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
olelod
wants to merge
5
commits into
main
Choose a base branch
from
test/neqsim-compatibility-suite
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
d850788
test(neqsim): add compatibility suite for the vendored jar
olelod 320cd07
test(libecalc): expand NeqSim compatibility suite with PH-flash and n…
olelod de2ec73
chore: bump NeqSim
olelod be72d37
chore: fix vulnerability
olelod e3fd0ab
docs: update neqsim_version_info.md for v3.17.0
olelod File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| name: NeqSim Compatibility Suite | ||
|
|
||
| # The NeqSim compatibility suite pins the behaviour of the vendored | ||
| # NeqSim jar (src/ecalc_neqsim_wrapper/lib/NeqSim.jar) over the | ||
| # operating envelope ecalc exercises. It is deselected from the default | ||
| # test run (see pyproject.toml `addopts`) and is intended to run only | ||
| # when the jar, the wrapper, or the suite itself changes. | ||
|
|
||
| on: | ||
| pull_request: | ||
| paths: | ||
| - 'src/ecalc_neqsim_wrapper/lib/NeqSim.jar' | ||
| - 'src/ecalc_neqsim_wrapper/lib/neqsim_version_info.md' | ||
| - 'src/ecalc_neqsim_wrapper/**.py' | ||
| - 'tests/ecalc_neqsim_wrapper/compatibility/**' | ||
| - '.github/workflows/test-neqsim-compatibility.yml' | ||
| workflow_dispatch: | ||
|
|
||
| permissions: { } | ||
|
|
||
| jobs: | ||
| enforce-version-note-update: | ||
| name: Enforce neqsim_version_info.md update when the jar changes | ||
| if: github.event_name == 'pull_request' | ||
| permissions: | ||
| contents: read | ||
| runs-on: ubuntu-24.04 | ||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| persist-credentials: false | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Refuse jar bump without version-note update | ||
| run: | | ||
| set -euo pipefail | ||
| base="origin/${{ github.base_ref }}" | ||
| changed=$(git diff --name-only "$base"...HEAD) | ||
| echo "Changed files:" | ||
| echo "$changed" | ||
| if echo "$changed" | grep -qx 'src/ecalc_neqsim_wrapper/lib/NeqSim.jar'; then | ||
| if ! echo "$changed" | grep -qx 'src/ecalc_neqsim_wrapper/lib/neqsim_version_info.md'; then | ||
| echo "::error::NeqSim.jar changed but neqsim_version_info.md was not updated." | ||
| exit 1 | ||
| fi | ||
| fi | ||
|
|
||
| test-neqsim-compatibility: | ||
| name: Run NeqSim compatibility suite | ||
| permissions: | ||
| contents: read | ||
| runs-on: ubuntu-24.04 | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| python-version: ["3.12", "3.14"] | ||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| persist-credentials: false | ||
|
|
||
| - name: Install uv | ||
| uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 | ||
| with: | ||
| enable-cache: true | ||
| python-version: ${{ matrix.python-version }} | ||
|
|
||
| - name: Set up Python | ||
| run: uv python install | ||
|
|
||
| - name: Install the project | ||
| run: uv sync --locked --all-extras --dev | ||
|
|
||
| - name: Report NeqSim jar fingerprint | ||
| run: | | ||
| ls -l src/ecalc_neqsim_wrapper/lib/NeqSim.jar | ||
| sha256sum src/ecalc_neqsim_wrapper/lib/NeqSim.jar | ||
| echo '--- neqsim_version_info.md ---' | ||
| cat src/ecalc_neqsim_wrapper/lib/neqsim_version_info.md || true | ||
|
|
||
| - name: Run NeqSim compatibility suite | ||
| timeout-minutes: 60 | ||
| run: | | ||
| uv run python -m pytest \ | ||
| -m neqsim_compat \ | ||
| tests/ecalc_neqsim_wrapper/compatibility/ \ | ||
| --junitxml=neqsim-compatibility.xml \ | ||
| -v | ||
|
|
||
| - name: Upload JUnit results | ||
| if: always() | ||
| uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 | ||
| with: | ||
| name: neqsim-compatibility-${{ matrix.python-version }} | ||
| path: neqsim-compatibility.xml | ||
| if-no-files-found: ignore | ||
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
Binary file not shown.
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,175 @@ | ||
| # NeqSim compatibility suite | ||
|
|
||
| NeqSim is a third-party Java library we ship as a jar file. That | ||
| arrangement is uncomfortable for several reasons. A new jar can shift the | ||
| value of a flash without anyone in the ecalc team being told. When a | ||
| NeqSim release regresses — i.e. starts returning different numbers than | ||
| it used to for inputs that previously worked — it usually does so | ||
| silently. The values look finite and reasonable, but they propagate | ||
| through compressor and pressure logic until something crashes far from | ||
| the root cause. And we query NeqSim in regions no one hand-checks: up | ||
| to 2000 bara at 450 K during max-speed probes, on whatever composition | ||
| the user feeds in. The existing application tests do not pin NeqSim's | ||
| outputs, so today the only signal that a jar bump broke something is | ||
| often an unrelated-looking failure several layers downstream. We would | ||
| like to stop being surprised this way. | ||
|
|
||
| A compatibility suite should give us three guarantees, in priority order. | ||
| A jar bump that changes any output we read should produce a failing test, | ||
| named after the (composition, P, T, EoS) state where it changed — not a | ||
| slow regression in some integration metric. A wrapper change that breaks | ||
| an operation we depend on (flashes, phase extraction, mixing, EoS | ||
| differentiation) should fail in the same checkout, not in the next ecalc | ||
| run. And an operating point ecalc can plausibly query, where NeqSim | ||
| returns garbage, should fail here before that garbage reaches the | ||
| compressor solver. | ||
|
|
||
| Equally important is what this suite is *not*. It is not an ecalc test | ||
| suite — bugs in solver logic, validation, units, caches, or process | ||
| units belong in the existing tests. It is not a NeqSim correctness suite | ||
| — we are detecting *change*, not certifying *physics*. And it is not run | ||
| on every PR; it is deselected by default and runs only when the jar, the | ||
| wrapper, or the suite itself changes, or when an engineer asks for it. | ||
|
|
||
| Structurally we want three layers, each answering a single question and | ||
| all reading from one source of truth for the operating envelope (pressure | ||
| range, temperature range, EoS models, compositions). The first layer asks | ||
| whether NeqSim returned a sensible number at all — finite, in physical | ||
| bounds, no stale defaults in the output — across the full envelope. The | ||
| second asks whether NeqSim plays by the rules under the operations we | ||
| depend on, with targeted tests that pin each operation's invariants rather | ||
| than its exact numerical output. The third asks whether any number we read | ||
| has drifted: a snapshot of every property ecalc reads at every state in | ||
| the envelope, generated against a known-good jar and reviewed on every | ||
| bump. | ||
|
|
||
| A failure must name the failing state in a form the engineer can paste | ||
| into a notebook to reproduce, and must make clear which layer caught it — | ||
| garbage value, broken invariant, or drifted number — without anyone | ||
| needing to read test source. | ||
|
|
||
| We accept upfront that this design will not catch ecalc-side bugs, bugs | ||
| at operating points outside the envelope, drift smaller than the snapshot | ||
| tolerance, or regressions in NeqSim operations the wrapper does not | ||
| exercise. Those trade-offs are deliberate. The suite is doing its job if | ||
| every jar bump PR ends in either a clean run, an explicitly accepted | ||
| snapshot diff, or a rejection of the bump — and if we stop finding NeqSim | ||
| regressions by way of downstream `NaN` crashes. | ||
|
|
||
| The GitHub Actions workflow `test-neqsim-compatibility.yml` gates on jar, | ||
| wrapper, suite, and workflow path changes. | ||
|
|
||
| ## Operating envelope | ||
|
|
||
| `envelope.py` is the single source of truth. Every state-generating | ||
| helper in this directory reads it from there: | ||
|
|
||
| | Dimension | Range | | ||
| | ------------- | ---------------------------------------------------------- | | ||
| | Pressure | 1 – 2000 bara (`MAX_FIRST_GUESS_BAR` cap on max-speed probes) | | ||
| | Temperature | 250 – 460 K (per-stage PH-flash outlet at off-design) | | ||
| | EoS | SRK, PR, GERG_SRK, GERG_PR | | ||
| | Operations | TP-flash, PH-flash, remove_liquid, mixing, property extraction | | ||
|
|
||
| Three named sub-grids cover three regimes ecalc realistically hits: | ||
|
|
||
| - `nominal_grid()` — 1–200 bara × 250–380 K (compressor suction to mid-discharge). | ||
| - `high_pressure_grid()` — 300–400 bara × 300–360 K (e.g. Sverdrup gas injection). | ||
| - `max_speed_probe_grid()` — 500–2000 bara × 400–450 K (dense-supercritical regime hit during max-speed probes). | ||
|
|
||
| When ecalc's envelope changes, update `envelope.py`. Everything | ||
| downstream re-derives. | ||
|
|
||
| ## How it's organised | ||
|
|
||
| Three groups, each answering a different question: | ||
|
|
||
| 1. **`sanity/` — does NeqSim return numbers that look real?** | ||
| Point-wise sanity checks (no NaN, no default kappa, values in | ||
| physical ranges) plus trajectory continuity (density monotonic in | ||
| P on single-phase segments, enthalpy monotonic in T at fixed P). | ||
| Also the external-reference checks for selected (composition, P, | ||
| T, EoS) states with published values, and the structural guard | ||
| that no test parametrises a wet composition below its temperature | ||
| floor. | ||
|
|
||
| 2. **`behaviour/` — does NeqSim follow the rules our pipeline code | ||
| assumes?** Self-consistency invariants for the operations ecalc | ||
| composes in production: state identities (round-trip, idempotency, | ||
| getter/setter consistency), phase operations (gas-phase extraction | ||
| matches a clean PT flash of the gas-phase composition), flash | ||
| operations (PH-flash returns the requested enthalpy, an enthalpy | ||
| increase raises temperature, a five-stage chain stays well-defined), | ||
| mixing (mass and molar balance), EoS differentiation (SRK vs PR | ||
| produce measurably different densities), and the wrapper's own | ||
| degenerate-state validators. | ||
|
|
||
| 3. **`regression/` — do the numbers match the previous jar?** A pinned | ||
| reference snapshot (`reference_snapshot.json`) holds every property | ||
| for every cell in the regression spec at strict 1e-9 relative | ||
| tolerance. Any drift fails the test; the fix is either to revert | ||
| the bump or to deliberately regenerate the snapshot with a brief | ||
| justification of the accepted drift. | ||
|
|
||
| ## Design constraints | ||
|
|
||
| These are baked into the state generators. The structural guard | ||
| `sanity/test_temperature_floors.py` will fail at collection time if a | ||
| new test or pin violates them. | ||
|
|
||
| - **No wet composition is flashed below ~273 K.** NeqSim does not | ||
| model the solid water phase. Below water's freezing point a flash | ||
| on a water-bearing composition can return NaN / default kappa / | ||
| non-physical Z, flip phase splits between EoS models, or in some | ||
| jar versions tear down the JVM gateway. The suite therefore | ||
| declines to test wet compositions below a conservative floor of | ||
| **280 K**. The floor lives in | ||
| `compositions.MIN_TEMPERATURE_KELVIN_PER_COMPOSITION` and is | ||
| populated automatically for any composition with `water > 0`. | ||
| Every state generator consults `is_state_supported(name, T)`. To | ||
| probe the cold/high-P region for a heavy composition that is | ||
| normally used wet, add a dry sister composition (see | ||
| `c3_rich_wellstream_dry`). | ||
|
|
||
| - **`sanity/` and `behaviour/` use lenient predicates; `regression/` | ||
| is strict.** The sanity and behaviour groups catch categorical | ||
| breakage (NaN, default, non-physical, broken algebraic identities). | ||
| Strict numerical pinning lives only in the regression snapshot. | ||
|
|
||
| ## Running it | ||
|
|
||
| ```bash | ||
| # Full suite, locally. Targeting the directory auto-enables the | ||
| # `neqsim_compat` marker; you don't need `-m neqsim_compat`. | ||
| uv run pytest tests/ecalc_neqsim_wrapper/compatibility/ | ||
|
|
||
| # Just the sanity group. | ||
| uv run pytest tests/ecalc_neqsim_wrapper/compatibility/sanity/ | ||
|
|
||
| # Regenerate the regression snapshot against the currently vendored jar. | ||
| uv run pytest tests/ecalc_neqsim_wrapper/compatibility/ --regenerate-neqsim-snapshot | ||
| ``` | ||
|
|
||
| The full suite takes around 20 minutes. JVM startup dominates the | ||
| first few seconds; the rest is NeqSim flash calls. | ||
|
|
||
| ## How to react to a failure | ||
|
|
||
| 1. **Sanity failure** — NeqSim is returning garbage at a state ecalc | ||
| exercises. Either the jar regressed in that region, or the suite | ||
| has hit a documented constraint that should be expressed in the | ||
| floor registry. Look at the failing state first; if it's a | ||
| wet composition below 273 K, the fix is in `compositions.py`, | ||
| not in NeqSim. | ||
|
|
||
| 2. **Behaviour failure** — NeqSim is returning numbers that violate | ||
| an algebraic identity ecalc relies on (e.g. PH-flash output | ||
| enthalpy doesn't match the input target). This is a real | ||
| regression and blocks the bump. | ||
|
|
||
| 3. **Regression failure** — A number drifted outside the strict | ||
| tolerance. Investigate before regenerating. If the drift is an | ||
| improvement (e.g. bug fix in a NeqSim correlation), regenerate | ||
| the snapshot and commit it alongside the jar bump with a brief | ||
| justification. If the drift is a quiet correctness loss, block | ||
| the bump. |
Empty file.
Empty file.
94 changes: 94 additions & 0 deletions
94
tests/ecalc_neqsim_wrapper/compatibility/behaviour/test_eos_differentiation.py
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| """EoS routing checks: SRK/PR differ, GERG setters refresh properties.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import pytest | ||
|
|
||
| from ecalc_neqsim_wrapper.thermo import NeqsimFluid | ||
| from libecalc.process.fluid_stream.fluid_model import EoSModel | ||
|
|
||
| from ..compositions import COMPOSITIONS | ||
|
|
||
| _GAS_DOMINANT_COMPOSITIONS = ( | ||
| "pure_methane", | ||
| "lean_natural_gas", | ||
| "typical_export_gas", | ||
| "rich_associated_gas", | ||
| "co2_heavy_injection", | ||
| "n2_heavy", | ||
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("composition_name", _GAS_DOMINANT_COMPOSITIONS) | ||
| def test_srk_and_pr_produce_measurably_different_densities(composition_name): | ||
| """SRK and PR are different cubic equations of state. At moderate | ||
| pressure they must produce measurably different (but close) | ||
| densities. Bit-identical output would mean the EoS selector is | ||
| wired wrong.""" | ||
| pressure_bara = 100.0 | ||
| temperature_kelvin = 320.0 | ||
| srk = NeqsimFluid.create_thermo_system( | ||
| composition=COMPOSITIONS[composition_name], | ||
| pressure_bara=pressure_bara, | ||
| temperature_kelvin=temperature_kelvin, | ||
| eos_model=EoSModel.SRK, | ||
| ) | ||
| pr = NeqsimFluid.create_thermo_system( | ||
| composition=COMPOSITIONS[composition_name], | ||
| pressure_bara=pressure_bara, | ||
| temperature_kelvin=temperature_kelvin, | ||
| eos_model=EoSModel.PR, | ||
| ) | ||
| density_diff = abs(srk.density - pr.density) / srk.density | ||
| assert density_diff > 1.0e-4, ( | ||
| f"SRK and PR densities are suspiciously close for {composition_name}: " | ||
| f"srk={srk.density!r} pr={pr.density!r} rel_diff={density_diff:.2e}" | ||
| ) | ||
| assert density_diff < 0.10, ( | ||
| f"SRK and PR densities differ by more than 10 % for {composition_name}: " | ||
| f"srk={srk.density!r} pr={pr.density!r} rel_diff={density_diff:.2%}" | ||
| ) | ||
| kappa_diff = abs(srk.kappa - pr.kappa) / srk.kappa | ||
| assert kappa_diff > 1.0e-4, ( | ||
| f"SRK and PR kappa are suspiciously close for {composition_name}: " | ||
| f"srk={srk.kappa!r} pr={pr.kappa!r} rel_diff={kappa_diff:.2e}" | ||
| ) | ||
| z_diff = abs(srk.z - pr.z) / srk.z | ||
| assert z_diff > 1.0e-4, ( | ||
| f"SRK and PR Z are suspiciously close for {composition_name}: srk={srk.z!r} pr={pr.z!r} rel_diff={z_diff:.2e}" | ||
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("eos_model", (EoSModel.GERG_SRK, EoSModel.GERG_PR)) | ||
| @pytest.mark.parametrize("composition_name", _GAS_DOMINANT_COMPOSITIONS) | ||
| def test_gerg_properties_refresh_after_setting_new_state(composition_name, eos_model): | ||
| """Under the GERG code path, properties cached at construction time | ||
| must reflect the new state after a TP setter — not the original | ||
| construction state.""" | ||
| state_a_p, state_a_t = 20.0, 310.0 | ||
| state_b_p, state_b_t = 80.0, 350.0 | ||
|
|
||
| direct_state_b = NeqsimFluid.create_thermo_system( | ||
| composition=COMPOSITIONS[composition_name], | ||
| pressure_bara=state_b_p, | ||
| temperature_kelvin=state_b_t, | ||
| eos_model=eos_model, | ||
| ) | ||
| via_setter = NeqsimFluid.create_thermo_system( | ||
| composition=COMPOSITIONS[composition_name], | ||
| pressure_bara=state_a_p, | ||
| temperature_kelvin=state_a_t, | ||
| eos_model=eos_model, | ||
| ).set_new_pressure_and_temperature(state_b_p, state_b_t) | ||
|
|
||
| for prop in ("density", "z", "kappa", "enthalpy_joule_per_kg"): | ||
| direct_value = getattr(direct_state_b, prop) | ||
| setter_value = getattr(via_setter, prop) | ||
| denom = max(abs(direct_value), 1.0) | ||
| relative_error = abs(direct_value - setter_value) / denom | ||
| assert relative_error < 1.0e-6, ( | ||
| f"{eos_model.name} cache appears stale: {prop} via setter " | ||
| f"({setter_value!r}) does not match direct construction " | ||
| f"at the same state ({direct_value!r}) for {composition_name}; " | ||
| f"rel_err={relative_error:.2e}" | ||
| ) |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.