[AI-Assisted] Add uncertainty-aware thermodynamic benchmark framework - #2632
Draft
EvenSol wants to merge 28 commits into
Draft
[AI-Assisted] Add uncertainty-aware thermodynamic benchmark framework#2632EvenSol wants to merge 28 commits into
EvenSol wants to merge 28 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a reusable thermodynamic benchmarking utility to compare NeqSim model predictions against published experimental data, including a first packaged H2–CO2 / H2–N2–CO2 phase-equilibrium dataset and accompanying documentation.
Changes:
- Introduces
ThermodynamicBenchmarkinfrastructure (immutable points/datasets, per-point rows, and aggregate error metrics including uncertainty-normalized residuals). - Adds a published Zhang et al. (2026) dataset as a packaged CSV resource + loader, and a NeqSim phase-equilibrium prediction adapter (SRK/PR/GERG-2008/GERG-2008-H2).
- Adds JUnit tests and reference-manual documentation/index entry for the new benchmark framework.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/neqsim/thermo/util/benchmark/ThermodynamicBenchmark.java | Core benchmark types (Point/Dataset/Row/Report) and aggregate metric calculation. |
| src/main/java/neqsim/thermo/util/benchmark/NeqSimPhaseEquilibriumPrediction.java | Adapter that computes bubble/dew-point pressures using selected NeqSim EOS configurations. |
| src/main/java/neqsim/thermo/util/benchmark/H2CO2PhaseEquilibriumData.java | Loader for the packaged Zhang 2026 H2–CO2 / H2–N2–CO2 dataset resource. |
| src/main/resources/data/thermo/benchmark/zhang2026_h2_co2_phase_equilibrium.csv | Machine-readable transcription of Tables IV–V experimental bubble/dew pressures. |
| src/test/java/neqsim/thermo/util/benchmark/ThermodynamicBenchmarkTest.java | Unit tests for dataset transcription/provenance, metrics, uncertainty residuals, and adapter configuration. |
| docs/thermo/thermodynamic_benchmarks.md | Documentation for benchmark framework + dataset scope + Java usage example. |
| docs/REFERENCE_MANUAL_INDEX.md | Adds reference-manual index entry for the new benchmarks page. |
Comments suppressed due to low confidence (1)
src/test/java/neqsim/thermo/util/benchmark/ThermodynamicBenchmarkTest.java:107
NeqSimPhaseEquilibriumPrediction's main behavior (predict) isn't exercised by the test suite yet (only the getter is). A small smoke test on one published point would catch API/signature regressions and basic model setup issues without asserting a specific numeric value.
Comment on lines
+68
to
+71
| if (!Double.isFinite(temperatureK) || temperatureK <= 0.0 | ||
| || !Double.isFinite(experimentalValue)) { | ||
| throw new IllegalArgumentException("Temperature and experimental value must be finite"); | ||
| } |
Comment on lines
+148
to
+151
| if (name == null || name.trim().isEmpty() || citation == null || doi == null | ||
| || license == null || points == null || points.isEmpty()) { | ||
| throw new IllegalArgumentException("Dataset metadata and points are required"); | ||
| } |
Comment on lines
+70
to
+73
| Property property = | ||
| "bubble".equals(values[2]) | ||
| ? Property.BUBBLE_POINT_PRESSURE | ||
| : Property.DEW_POINT_PRESSURE; |
Comment on lines
+45
to
+52
| ```java | ||
| ThermodynamicBenchmark.Dataset dataset = H2CO2PhaseEquilibriumData.load(); | ||
| NeqSimPhaseEquilibriumPrediction prediction = | ||
| new NeqSimPhaseEquilibriumPrediction( | ||
| NeqSimPhaseEquilibriumPrediction.Model.GERG_2008_H2); | ||
|
|
||
| ThermodynamicBenchmark.Report report = | ||
| ThermodynamicBenchmark.run("GERG-2008-H2", dataset, prediction); |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| if (values.length != 8) { | ||
| throw new IOException("Expected 8 CSV columns but found " + values.length); | ||
| } | ||
| double temperatureK = Double.parseDouble(values[1]) + 273.15; |
| throw new IOException("Expected 8 CSV columns but found " + values.length); | ||
| } | ||
| double temperatureK = Double.parseDouble(values[1]) + 273.15; | ||
| double pressureBara = Double.parseDouble(values[3]) * 10.0; |
| Map<String, Double> composition = new LinkedHashMap<String, Double>(); | ||
| composition.put("CO2", Double.parseDouble(values[4])); | ||
| composition.put("hydrogen", Double.parseDouble(values[5])); | ||
| double nitrogenFraction = Double.parseDouble(values[6]); |
| double temperatureK = Double.parseDouble(values[1]) + 273.15; | ||
| double pressureBara = Double.parseDouble(values[3]) * 10.0; | ||
| Map<String, Double> composition = new LinkedHashMap<String, Double>(); | ||
| composition.put("CO2", Double.parseDouble(values[4])); |
| double pressureBara = Double.parseDouble(values[3]) * 10.0; | ||
| Map<String, Double> composition = new LinkedHashMap<String, Double>(); | ||
| composition.put("CO2", Double.parseDouble(values[4])); | ||
| composition.put("hydrogen", Double.parseDouble(values[5])); |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (4)
src/main/java/neqsim/thermo/util/benchmark/ThermodynamicBenchmark.java:70
- Point currently validates temperature and experimentalValue, but does not validate pressureBara (can be NaN/<=0) or standardUncertainty, and allows experimentalValue==0 even though Row computes relative errors by dividing by experimentalValue (→ Infinity/NaN). Consider enforcing finite positive pressure, non-zero experimentalValue, and non-negative (or NaN) uncertainty to keep reports well-defined.
if (!Double.isFinite(temperatureK) || temperatureK <= 0.0 || !Double.isFinite(experimentalValue)) {
throw new IllegalArgumentException("Temperature and experimental value must be finite");
}
double compositionSum = 0.0;
src/main/java/neqsim/thermo/util/benchmark/ThermodynamicBenchmark.java:149
- Dataset constructor rejects nulls and empty name, but allows blank citation/doi/license strings (e.g., ""). Since these fields are meant for provenance/auditability, it’s safer to enforce non-blank values consistently.
public Dataset(String name, String citation, String doi, String license, List<Point> points) {
if (name == null || name.trim().isEmpty() || citation == null || doi == null || license == null || points == null
|| points.isEmpty()) {
throw new IllegalArgumentException("Dataset metadata and points are required");
}
src/main/java/neqsim/thermo/util/benchmark/H2CO2PhaseEquilibriumData.java:68
- CSV parsing currently treats any non-"bubble" token as dew-point, which can silently misclassify data if the file contains typos/whitespace/unexpected values. It’s safer to trim and validate explicitly, failing fast on unknown property values.
Property property = "bubble".equals(values[2]) ? Property.BUBBLE_POINT_PRESSURE : Property.DEW_POINT_PRESSURE;
points.add(new Point(property, temperatureK, pressureBara, pressureBara, Double.NaN, "bara", composition));
docs/thermo/thermodynamic_benchmarks.md:56
- This documentation adds a Java code example, but it doesn’t appear to be exercised by the existing documentation-compilation tests. In this repo, Java doc examples are typically kept compiling/running via DocExamplesCompilationTest; without that, the snippet can silently rot.
```java
ThermodynamicBenchmark.Dataset dataset = H2CO2PhaseEquilibriumData.load();
NeqSimPhaseEquilibriumPrediction prediction =
new NeqSimPhaseEquilibriumPrediction(
NeqSimPhaseEquilibriumPrediction.Model.GERG_2008_H2);
ThermodynamicBenchmark.Report report =
ThermodynamicBenchmark.run("GERG-2008-H2", dataset, prediction);
double aardPercent = report.getAverageAbsoluteRelativeDeviationPercent();
double biasPercent = report.getBiasPercent();
</details>
Comment on lines
+66
to
+72
| double pressureBara = system.getPressure("bara"); | ||
| if (!Double.isFinite(pressureBara) || pressureBara <= 0.0) { | ||
| throw new IllegalStateException( | ||
| model + " returned invalid " + point.getProperty() + " at " + point.getTemperatureK() + " K"); | ||
| } | ||
| return pressureBara; | ||
| } |
| Dataset dataset = new Dataset("uncertainty test", "test citation", "10.0000/test", "test data", | ||
| java.util.Collections.singletonList(point)); | ||
|
|
||
| Report report = ThermodynamicBenchmark.run("test model", dataset, value -> 37.5); |
Comment on lines
+75
to
+82
| @Test | ||
| void exposesConfiguredNeqSimModel() { | ||
| NeqSimPhaseEquilibriumPrediction prediction = new NeqSimPhaseEquilibriumPrediction( | ||
| NeqSimPhaseEquilibriumPrediction.Model.GERG_2008_H2); | ||
|
|
||
| assertEquals(NeqSimPhaseEquilibriumPrediction.Model.GERG_2008_H2, prediction.getModel()); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
src/main/java/neqsim/thermo/util/benchmark/H2CO2PhaseEquilibriumData.java:68
- CSV property parsing currently treats any value other than the exact string "bubble" as dew-point. That can silently misclassify data if the column contains unexpected values (e.g., whitespace, capitalization, typos). Parse both bubble/dew explicitly and throw an IOException for unknown values.
Property property = "bubble".equals(values[2]) ? Property.BUBBLE_POINT_PRESSURE : Property.DEW_POINT_PRESSURE;
points.add(new Point(property, temperatureK, pressureBara, pressureBara, Double.NaN, "bara", composition));
src/main/java/neqsim/thermo/util/benchmark/ThermodynamicBenchmark.java:69
- Point validation does not check pressure/uncertainty, and allows an experimental value of 0.0 even though the benchmark metrics (relative error) divide by the experimental value. This can lead to invalid states (NaN/Inf relative errors or invalid initial pressures). Validate pressure as finite/positive, require experimentalValue to be non-zero, and require standardUncertainty to be positive when provided.
if (!Double.isFinite(temperatureK) || temperatureK <= 0.0 || !Double.isFinite(experimentalValue)) {
throw new IllegalArgumentException("Temperature and experimental value must be finite");
}
docs/thermo/thermodynamic_benchmarks.md:49
- This page adds a runnable Java example, but there is no corresponding compilation/execution coverage in the documentation example test suite (DocExamplesCompilationTest). Without a test, the snippet can drift and break without CI catching it. Add a DocExamplesCompilationTest case that exercises the same API calls shown here.
```java
ThermodynamicBenchmark.Dataset dataset = H2CO2PhaseEquilibriumData.load();
NeqSimPhaseEquilibriumPrediction prediction =
new NeqSimPhaseEquilibriumPrediction(
NeqSimPhaseEquilibriumPrediction.Model.GERG_2008_H2);
Comment on lines
+140
to
+148
| throw new AssertionError( | ||
| "CALIBRATION SRK kij=" | ||
| + srk.getBinaryInteractionParameter() | ||
| + " RMSRE=" | ||
| + srk.getRootMeanSquareRelativeErrorPercent() | ||
| + "; PR kij=" | ||
| + pr.getBinaryInteractionParameter() | ||
| + " RMSRE=" | ||
| + pr.getRootMeanSquareRelativeErrorPercent()); |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
src/test/java/neqsim/thermo/util/benchmark/ThermodynamicBenchmarkTest.java:114
- This test will always fail because it unconditionally throws an AssertionError after running the calibration loop. That breaks CI and also makes the test suite non-deterministic/debug-only.
throw new AssertionError("CALIBRATION SRK kij=" + srk.getBinaryInteractionParameter() + " RMSRE="
+ srk.getRootMeanSquareRelativeErrorPercent() + "; PR kij=" + pr.getBinaryInteractionParameter() + " RMSRE="
+ pr.getRootMeanSquareRelativeErrorPercent());
src/main/java/neqsim/thermo/util/benchmark/ThermodynamicBenchmark.java:69
- Point validation allows non-finite/invalid pressures and an experimental value of 0.0. Downstream calculations (relative error in Row and objective()) divide by experimentalValue, so a zero value would yield Infinity/NaN and corrupt aggregate metrics.
if (!Double.isFinite(temperatureK) || temperatureK <= 0.0 || !Double.isFinite(experimentalValue)) {
throw new IllegalArgumentException("Temperature and experimental value must be finite");
}
src/main/java/neqsim/thermo/util/benchmark/H2CO2PhaseEquilibriumData.java:68
- CSV parsing treats any property value other than the exact string "bubble" as a dew point. That can silently misclassify points if the file contains unexpected values (typos, capitalization) and hide transcription errors.
Property property = "bubble".equals(values[2]) ? Property.BUBBLE_POINT_PRESSURE : Property.DEW_POINT_PRESSURE;
points.add(new Point(property, temperatureK, pressureBara, pressureBara, Double.NaN, "bara", composition));
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
src/main/java/neqsim/thermo/util/benchmark/ThermodynamicBenchmark.java:69
Pointaccepts invalid physical state and uncertainty values (e.g., non-finite/negative pressure, negative uncertainty) and allowsexperimentalValue == 0, which will later cause a division-by-zero when computing relative error inRow. SincePointis the public immutable input type, validatepressureBaraandstandardUncertaintyand reject a zero experimental value up front.
if (property == null || unit == null || unit.trim().isEmpty() || composition == null || composition.isEmpty()) {
throw new IllegalArgumentException("Property, unit, and composition are required");
}
if (!Double.isFinite(temperatureK) || temperatureK <= 0.0 || !Double.isFinite(experimentalValue)) {
throw new IllegalArgumentException("Temperature and experimental value must be finite");
}
src/main/java/neqsim/thermo/util/benchmark/H2CO2PhaseEquilibriumData.java:68
- CSV parsing maps any non-"bubble" value to dew-point pressure, which can silently mask transcription errors (e.g., unexpected strings, casing, whitespace). Validate the
propertycolumn explicitly and fail fast with anIOExceptionwhen it is not "bubble" or "dew".
Property property = "bubble".equals(values[2]) ? Property.BUBBLE_POINT_PRESSURE : Property.DEW_POINT_PRESSURE;
points.add(new Point(property, temperatureK, pressureBara, pressureBara, Double.NaN, "bara", composition));
src/test/java/neqsim/thermo/util/benchmark/ThermodynamicBenchmarkTest.java:137
- This test unconditionally fails via
throw new AssertionError(...), which will breakmvn testand the newkij-calibrationworkflow. Replace the unconditional failure with logging (Log4j2) and at least minimal assertions that keep the test meaningful while allowing CI to pass.
new NeqSimPhaseEquilibriumPrediction(NeqSimPhaseEquilibriumPrediction.Model.SRK));
Report prDefault = ThermodynamicBenchmark.run("PR default", binary,
new NeqSimPhaseEquilibriumPrediction(NeqSimPhaseEquilibriumPrediction.Model.PR));
throw new AssertionError("CALIBRATION SRK kij=" + srk.getBinaryInteractionParameter() + " RMSRE="
+ srk.getRootMeanSquareRelativeErrorPercent() + " trainAARD="
+ srkTrain.getAverageAbsoluteRelativeDeviationPercent() + " validationAARD="
src/test/java/neqsim/thermo/util/benchmark/ThermodynamicBenchmarkTest.java:84
- The docs claim support for GERG-2008 / GERG-2008-H2 predictions, but the tests only assert the enum value (and that custom kij is rejected). Add a small smoke test that actually runs the GERG-2008-H2 adapter on at least one published point and asserts the prediction is finite, so documentation/API claims are exercised by CI.
void exposesConfiguredNeqSimModel() {
NeqSimPhaseEquilibriumPrediction prediction = new NeqSimPhaseEquilibriumPrediction(
NeqSimPhaseEquilibriumPrediction.Model.GERG_2008_H2);
assertEquals(NeqSimPhaseEquilibriumPrediction.Model.GERG_2008_H2, prediction.getModel());
}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
src/main/java/neqsim/thermo/util/benchmark/ThermodynamicBenchmark.java:69
- Point validation currently allows non-finite/negative pressure, zero experimental values, and negative standard uncertainty. This can produce NaN/Infinity errors later (relative-error calculations divide by experimentalValue, and prediction adapters rely on a valid state). Consider rejecting invalid pressure, disallowing experimentalValue==0 when using relative metrics, and enforcing non-negative uncertainty when provided.
if (!Double.isFinite(temperatureK) || temperatureK <= 0.0 || !Double.isFinite(pressureBara)
|| pressureBara <= 0.0 || !Double.isFinite(experimentalValue) || experimentalValue == 0.0) {
throw new IllegalArgumentException("Temperature, pressure, and experimental value must be physical and finite");
src/main/java/neqsim/thermo/util/benchmark/H2CO2PhaseEquilibriumData.java:68
- CSV parsing treats any non-"bubble" token as a dew point (including typos/whitespace), which can silently corrupt the dataset. It would be safer to trim + validate the token and fail fast on unexpected values.
Property property;
if ("bubble".equals(values[2])) {
src/test/java/neqsim/thermo/util/benchmark/ThermodynamicBenchmarkTest.java:81
- The documentation and API surface introduce NeqSimPhaseEquilibriumPrediction as a runnable adapter, but the tests only assert configuration (getModel/getKij) and never call predict(). Adding a single-point prediction test would catch regressions in flash setup/mixing-rule selection and ensure the adapter remains executable.
@Test
void rejectsInvalidStateValueAndUncertainty() {
Map<String, Double> composition = new LinkedHashMap<String, Double>();
composition.put("CO2", 0.96);
composition.put("hydrogen", 0.04);
assertThrows(IllegalArgumentException.class,
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
src/main/java/neqsim/thermo/util/benchmark/ThermodynamicBenchmark.java:159
- Dataset constructor allows empty/blank citation/doi/license strings and does not validate that the points list contains no null entries. That can lead to NPEs later (e.g., in run/report) and contradicts the "metadata and points are required" contract.
public Dataset(String name, String citation, String doi, String license, List<Point> points) {
if (name == null || name.trim().isEmpty() || citation == null || doi == null || license == null || points == null
|| points.isEmpty()) {
throw new IllegalArgumentException("Dataset metadata and points are required");
}
this.name = name;
this.citation = citation;
this.doi = doi;
this.license = license;
this.points = Collections.unmodifiableList(new ArrayList<Point>(points));
}
docs/thermo/thermodynamic_benchmarks.md:56
- The documentation includes a Java code example, but it is not covered by the repository’s existing "doc examples compile" convention (src/test/java/neqsim/DocExamplesCompilationTest.java). Adding a dedicated test method there for this snippet would help ensure future API changes don’t silently break the published documentation.
## Java example
```java
ThermodynamicBenchmark.Dataset dataset = H2CO2PhaseEquilibriumData.load();
NeqSimPhaseEquilibriumPrediction prediction =
new NeqSimPhaseEquilibriumPrediction(
NeqSimPhaseEquilibriumPrediction.Model.GERG_2008_H2);
ThermodynamicBenchmark.Report report =
ThermodynamicBenchmark.run("GERG-2008-H2", dataset, prediction);
double aardPercent = report.getAverageAbsoluteRelativeDeviationPercent();
double biasPercent = report.getBiasPercent();
</details>
Comment on lines
+54
to
+63
| String[] values = line.split(",", -1); | ||
| if (values.length != 8) { | ||
| throw new IOException("Expected 8 CSV columns but found " + values.length); | ||
| } | ||
| double temperatureK = Double.parseDouble(values[1]) + 273.15; | ||
| double pressureBara = Double.parseDouble(values[3]) * 10.0; | ||
| Map<String, Double> composition = new LinkedHashMap<String, Double>(); | ||
| composition.put("CO2", Double.parseDouble(values[4])); | ||
| composition.put("hydrogen", Double.parseDouble(values[5])); | ||
| double nitrogenFraction = Double.parseDouble(values[6]); |
This was referenced Jul 27, 2026
This was referenced Aug 10, 2026
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.
Summary
Adds a reusable, uncertainty-aware experimental-data benchmark framework, a bounded constant-kij fitter, and the first published H2-CO2/CO2-H2-N2 phase-equilibrium dataset.
Capability
Dataset and fit
The fit used bounds [-0.5, 0.2], tolerance 1e-4, and at most 20 objective evaluations. It reduces the selected objective but remains technically unacceptable as a general model. The fitted constants are documented for reproducibility and are deliberately not installed as database defaults.
Validation
Scope boundary
PRSV is not included because NeqSim currently exposes no verified PRSV system implementation. The poor constant-kij holdout results indicate that point-level residuals, flash formulation, temperature dependence, and independent binary data should be investigated before any interaction-parameter database change.
Model predictions are calculated independently for every experimental state. Experimental pressure is only the numerical initial pressure and is never substituted as a prediction.