Skip to content

feat(svdsbtl): split SUPER at IF97 R2/R3 boundary + fix R3 sampling#2938

Merged
ibell merged 2 commits into
masterfrom
ihb/svdsbtl-split-b23
May 21, 2026
Merged

feat(svdsbtl): split SUPER at IF97 R2/R3 boundary + fix R3 sampling#2938
ibell merged 2 commits into
masterfrom
ihb/svdsbtl-split-b23

Conversation

@ibell
Copy link
Copy Markdown
Contributor

@ibell ibell commented May 20, 2026

Summary

Closes the biggest IAPWS G13-15 conformance gap for SVDSBTL&IF97 by putting the IF97 R2/R3 derivative kink on a region edge instead of inside a cell.

The IF97 R2/R3 boundary curve p = p_B23(T) crosses our SUPER region's interior. IF97 uses different basis equations on the two sides — property values match at the boundary but derivatives have a kink. Our single-region SVD encoded the discontinuity into its modes; Hermite-cubic interpolation overshot; R3 worst-case residuals against IAPWS-IF97 were ~190 K in T and ~110% in v.

Subagent investigation (CoolProp-foi.9.5, 2026-05-20) ruled out kernel and rank alternatives — a smoother kernel can't reconstruct a discontinuous function. Three independent agents converged on this atlas-level fix.

Changes

  1. SUPER region split in src/SBTL/SurfacePresets.cpp for IF97 source: SUPER_R3 (h < h_B23(p)) + SUPER_R2 (h > h_B23(p)) over p ∈ [pcrit, 100 MPa]. SUPER_HIGH_P single-region above 100 MPa (B23 curve not defined there). HEOS source unchanged.
  2. h_B23(p) boundary curve via build_h_B23_curve helper: 64 log-spaced p knots, IF97 forward h at (T_B23(p), p), fitted with existing region::CubicSplineCurve machinery.
  3. R3 sampling fix: IF97's HmassP_INPUTS doesn't implement R3 (no closed-form backward), throws "Pressure out of range". Previously the Newton-refining lambda propagated the throw, and sample_grid NaN-fill / median-patched the bad cells — masked by the old wide SUPER, catastrophic with the smaller SUPER_R3. Fix: catch the HmassP throw and seed Newton from T = 700 K on PT_INPUTS.
  4. Conformance probe src/Tests/CoolProp-Tests-SVDSBTLFailMap.cpp ([!benchmark]) — used during the investigation, lands as the conformance gate for follow-up PRs.
  5. Cache rev 6 → 7.

Measured impact (SVDSBTL&IF97 vs IF97, NT=200, NR=800, rank=20)

metric before after ratio
R1 v_max 1.2774% 0.1640% 8× better
R1 w_max 3.7681% 0.0806% 47× better
R2 v_max 11.5399% 0.0089% 1300× better
R2 s_max 162.4 J/kgK 0.13 J/kgK 1290× better
R2 T_max 10928 mK 17.8 mK 615× better
R3 v_max 112% 213%* (one critical-zone outlier; see below)
R3 T_max 188682 mK 81352 mK*

* R3 has a single remaining outlier at T=663.7 K, p=26.6 MPa — just outside the default critical-patch bbox at [0.95, 1.05]·Tcrit × [0.75, 1.15]·pcrit = [614.7, 679.5] K × [16.55, 25.37] MPa. Widening that bbox (PR D / CoolProp-dxd) absorbs it. Excluding this one near-critical probe, R3 worst-case T drops 350× (188 K → 530 mK), v drops to 0.34% range.

Follow-ups in the foi.9 sequence

  • foi.9.6 quintic Hermite kernel — tighten R1/R2 fully to G13-15 perms
  • foi.9.7 IF97-boundary smoothing of M (Kunick §5.1 trick) — optional polish
  • foi.5ni / foi.dxd — auto-calibrate the critical-patch bbox

Test plan

  • [SVDSBTL][pt][water] HEOS source unchanged (48 s, all assertions pass)
  • [SVDSBTL][ph][water] HEOS source unchanged (warm cache, <1 s)
  • SVDSBTL&IF97 single-phase PT lookup matches IF97 truth passes
  • Fail-map CSV regenerated and tabulated above
  • Full [SVDSBTL] suite — CI

Refs: CoolProp-foi.9.5

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved accuracy for water (IF97) thermodynamic properties near the R2/R3 region boundary by refining surface handling to avoid interpolation artifacts.
  • Tests

    • Added a new diagnostic test that generates CSV “fail-map” data for conformance/validation comparisons across backends.
  • Chores

    • Cache format bumped; previously cached surface data will be regenerated on first use.

Review Change Stack

The IAPWS-IF97 R2/R3 boundary curve p = p_B23(T) crosses the SBTL
SUPER region's interior.  IF97 switches between basis equations
across the curve: R2 uses the g-basis polynomial, R3 uses the
ρ-T fundamental Helmholtz.  Property *values* match at the
boundary by IF97 construction, but *derivatives* have a kink.
Our single-region SVD over the kink-crossing SUPER region was
encoding the discontinuity into the modes and the Hermite-cubic
interpolation overshot, producing R3 worst-case residuals against
IAPWS-IF97 of ~190 K in T and ~110% in v — five orders of magnitude
beyond G13-15 perm.

Subagent investigation 2026-05-20 (CoolProp-foi.9.5) ruled out
basis-function and rank-truncation alternatives.  Three independent
agents converged on the same diagnosis: a smoother kernel can't
reconstruct a discontinuous function.  Fix the kink at the atlas
level by putting it on a region edge rather than inside a cell.

This commit:

1. Splits SUPER into SUPER_R3 (h < h_B23(p)) and SUPER_R2
   (h > h_B23(p)) for IF97 source backend over p ∈ [pcrit, 100 MPa]
   (the valid range of the IF97 B23 curve).  For p > 100 MPa (above
   the curve's defined range) a single SUPER_HIGH_P region covers
   the extrapolation slab.  HEOS source unchanged (no piecewise EOS
   to align against).
2. Adds if97_T_B23_K and build_h_B23_curve helpers in SurfacePresets.cpp
   that fit a CubicSplineCurve through 64 log-spaced p knots.
3. Fixes a pre-existing IF97-sampling bug: IF97's HmassP_INPUTS
   throws "Pressure out of range" in R3 territory (no closed-form
   backward equation), which the previous Newton-refining lambda
   would propagate.  Sample_grid's NaN-fill / median-patch was
   silently filling R3 cells with garbage — masked by the old wide
   SUPER region but catastrophic with the smaller SUPER_R3.  Fix:
   catch the HmassP throw and start Newton from T = 700 K (mid-R3
   T range) on PT_INPUTS instead.
4. Adds src/Tests/CoolProp-Tests-SVDSBTLFailMap.cpp — a [!benchmark]-
   tagged CSV-emitting conformance probe used during the
   investigation; lands as the conformance gate for future tightening
   PRs in the foi.9 sequence.
5. Bumps cache rev 6 → 7.  Region count for IF97-source water
   surfaces changes 3 → 4 or 5, so the on-wire region list differs
   and rev-6 caches must rebuild.

Conformance impact (SVDSBTL&IF97 vs IF97, NT=200, NR=800, rank=20,
G13-15 IF97 budgets: T<25 mK R1/R3, <10 mK R2/R5; v,s,w<0.001% / 1e-3 J/kgK):

                       baseline    after PR
R1 v max               1.2774%     0.1640%   (8× better)
R1 w max               3.7681%     0.0806%   (47× better)
R1 s max               18.16       2.52      (7× better)
R2 v max              11.5399%     0.0089%   (1300× better)
R2 s max             162.4480      0.1262    (1290× better)
R2 T max          10927.74 mK     17.76 mK   (615× better)
R3 v max             112.0231%   213.12%a    (one critical-zone outlier
R3 T max         188682.10 mK  81351.84 mKa   left; rest of R3 now in
R3 s max            1924.17      1282.63a    the 0.02-0.35% range,
R3 w max             177.65%      56.21%a    T < 530 mK)
R5                  all 0         all 0     (was already clean)

a) The remaining R3 outlier (T=663.7 K, p=26.6 MPa) sits just outside
the default critical-patch bbox at [0.95, 1.05]·Tcrit × [0.75, 1.15]·pcrit
= [614.7, 679.5] K × [16.55, 25.37] MPa.  Widening the patch (PR D —
CoolProp-dxd — auto-calibration loop) absorbs it.  Excluding that one
near-critical outlier, R3's worst T residual drops 350× (188 K → 530 mK).

Follow-ups in the foi.9 sequence:
- foi.9.6 (quintic Hermite kernel) — tighten R1/R2 to G13-15 perms
- foi.9.7 (Kunick §5.1 IF97-boundary smoothing of M) — optional polish
- foi.5ni / foi.dxd — auto-calibrate critical-patch bbox to absorb the
  remaining R3 near-critical outlier

Refs: CoolProp-foi.9.5

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4691607e-4cc9-43c0-865b-f1ebb4b2be35

📥 Commits

Reviewing files that changed from the base of the PR and between 6905d36 and 79eb9aa.

📒 Files selected for processing (1)
  • src/Tests/CoolProp-Tests-SVDSBTLFailMap.cpp

📝 Walkthrough

Walkthrough

Splits IF97 SUPER along the B23 boundary to avoid R2/R3 interpolation kinks, adds B23 utilities, hardens PH query logic with an exception-safe fallback, bumps the SVDSurfaceSerializer revision to 7, and adds CSV “fail-map” conformance tests for SVDSBTL vs IF97/HEOS.

Changes

IF97 Region Boundary Fix and Validation

Layer / File(s) Summary
Serializer cache invalidation marker
include/CoolProp/sbtl/SVDSurfaceSerializer.h
Revision constant bumped from 6 to 7 with documentation explaining that IF97 SUPER region split changes stored region and boundary contents, forcing cache invalidation.
IF97 B23 boundary utilities and SUPER region splitting
src/SBTL/SurfacePresets.cpp
Adds closed-form B23 boundary inverse and cubic-spline sampling; modifies ph_subcritical to detect IF97 source and B23 overlap, splitting SUPER into multiple regions using the B23 spline as a boundary.
PH query robustness and exception safety
src/SBTL/SurfacePresets.cpp
Refactors IF97 update_state lambda to try HmassP_INPUTS with phase detection and single-phase pinning, and falls back to bracketed Newton iteration on exception; ensures explicit phase unpinning in both paths.
Conformance test framework
src/Tests/CoolProp-Tests-SVDSBTLFailMap.cpp
Implements region sampling, truth-state forward-h refinement, SVDSBTL vs reference runs, and CSV residual metrics; registers three benchmark-tagged test cases for SVDSBTL/IF97 and SVDSBTL/HEOS comparisons.
Test build integration
CMakeLists.txt
Adds src/Tests/CoolProp-Tests-SVDSBTLFailMap.cpp to APP_SOURCES when COOLPROP_CATCH_MODULE is enabled so the test is compiled into the Catch test runner.

Poem

I’m a rabbit with a spline and a chart,
I split SUPER where IF97 parts part;
I pin, then unpin, if errors ensue,
I run fail-maps and print rows anew.
Hops, CSVs, and a careful heart 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: splitting the SUPER region at the IF97 R2/R3 boundary and fixing R3 sampling issues, which are the primary objectives of the PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ihb/svdsbtl-split-b23

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/SBTL/SurfacePresets.cpp (1)

279-292: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail the fallback if Newton never reaches the requested enthalpy.

This loop can stop because cp is non-finite/non-positive or just exhaust 10 iterations, and Lines 292-293 then return whatever state is left. That silently serializes off-manifold samples into the cache. Please verify the final residual and throw on non-convergence instead of accepting the last iterate.

🔧 Minimal guard for silent non-convergence
             try {
+                const double tol = 1e-10 * std::max(1.0, std::abs(h));
+                bool converged = false;
                 for (int k = 0; k < 10; ++k) {
                     const double h_now = s.hmass();
                     const double dh = h_now - h;
-                    if (std::abs(dh) < 1e-10 * std::abs(h)) break;
+                    if (std::abs(dh) < tol) {
+                        converged = true;
+                        break;
+                    }
                     const double cp = s.cpmass();
-                    if (!std::isfinite(cp) || cp <= 0.0) break;
+                    if (!std::isfinite(cp) || cp <= 0.0) break;
                     const double T_new = s.T() - dh / cp;
                     s.update(::CoolProp::PT_INPUTS, p, T_new);
                 }
+                if (!converged && std::abs(s.hmass() - h) >= tol) {
+                    throw std::runtime_error("IF97 PH fallback did not converge");
+                }
             } catch (...) {
                 if (pinned) s.unspecify_phase();
                 throw;
             }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/SBTL/SurfacePresets.cpp` around lines 279 - 292, The Newton loop that
adjusts enthalpy may exit due to non-finite cp or max iterations without
reaching the target h, but the current code accepts the last iterate; change
this by computing the final residual (dh = s.hmass() - h) after the loop and if
std::abs(dh) >= 1e-10 * std::abs(h) (or cp non-finite/<=0 caused an early break)
throw a descriptive exception (e.g. std::runtime_error) so the fallback fails;
keep the existing pinned cleanup (s.unspecify_phase()) in both the error and
success paths (use the same pinned variable and the existing try/catch pattern)
and reference the variables/functions s.hmass(), s.cpmass(),
s.update(::CoolProp::PT_INPUTS, p, T_new), s.unspecify_phase(), h, p to locate
where to add the post-loop residual check and throw.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/SBTL/SurfacePresets.cpp`:
- Around line 265-270: Replace the broad catch(...) around the
s.update(::CoolProp::PT_INPUTS, p, 700.0) fallback with a narrow catch for the
specific CoolProp exception that indicates the missing-R3-backward/path case
(e.g., catch(const CoolProp::ValueError &e)) and perform the 700 K seed only in
that handler; add a general catch(...) that rethrows (throw;) so unrelated
exceptions are not swallowed. Ensure references to the existing
s.update(::CoolProp::PT_INPUTS, p, 700.0) remain unchanged inside the specific
handler.

In `@src/Tests/CoolProp-Tests-SVDSBTLFailMap.cpp`:
- Around line 120-147: The CSV currently prints the sampled T variable while the
temperature residual dT_mK is computed against T_truth_refined; update the
printf call that emits the CSV row (the line printing region,h_truth,p_Pa,T,...)
to output T_truth_refined (the refined truth temperature produced by
refine_to_forward_h and stored in T_truth_refined) instead of the original
sampled T so the T_truth_K column matches the dT_mK calculation.
- Around line 6-7: The header example uses a non-existent tag "[heos_src]" so
update the example to the actual test tag (replace "[heos_src]" with the correct
tag, e.g. "[heos]") so the documented CatchTestRunner command selects the HEOS
tests; edit the two example lines in the header comment in
CoolProp-Tests-SVDSBTLFailMap.cpp accordingly.

---

Outside diff comments:
In `@src/SBTL/SurfacePresets.cpp`:
- Around line 279-292: The Newton loop that adjusts enthalpy may exit due to
non-finite cp or max iterations without reaching the target h, but the current
code accepts the last iterate; change this by computing the final residual (dh =
s.hmass() - h) after the loop and if std::abs(dh) >= 1e-10 * std::abs(h) (or cp
non-finite/<=0 caused an early break) throw a descriptive exception (e.g.
std::runtime_error) so the fallback fails; keep the existing pinned cleanup
(s.unspecify_phase()) in both the error and success paths (use the same pinned
variable and the existing try/catch pattern) and reference the
variables/functions s.hmass(), s.cpmass(), s.update(::CoolProp::PT_INPUTS, p,
T_new), s.unspecify_phase(), h, p to locate where to add the post-loop residual
check and throw.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 41517523-78f5-4d34-9541-b3c978d8562c

📥 Commits

Reviewing files that changed from the base of the PR and between 2c4c34f and 6905d36.

📒 Files selected for processing (4)
  • CMakeLists.txt
  • include/CoolProp/sbtl/SVDSurfaceSerializer.h
  • src/SBTL/SurfacePresets.cpp
  • src/Tests/CoolProp-Tests-SVDSBTLFailMap.cpp

Comment thread src/SBTL/SurfacePresets.cpp
Comment thread src/Tests/CoolProp-Tests-SVDSBTLFailMap.cpp Outdated
Comment thread src/Tests/CoolProp-Tests-SVDSBTLFailMap.cpp Outdated
Two CodeRabbit comments on PR #2938:

1. The header-comment example used the tag `[heos_src]`, which matches
   no test case — the actual tags are `[heos_src_vs_if97]` and
   `[heos_src_vs_heos]`.  Updated to list all three invocations
   accurately.

2. The CSV column `T_truth_K` was being filled with the *sampled* T,
   but the `dT_mK` residual is computed against `T_truth_refined`
   (the post-Newton-refinement T).  This made `T_truth_K` misleading
   — analysis scripts treating it as the actual truth-side T would
   see a value that differs from the residual's reference frame.
   Switched the column to write `T_truth_refined`.

No behavioral change to SVDSBTL itself; only fixes downstream
analysis correctness.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ibell
Copy link
Copy Markdown
Contributor Author

ibell commented May 21, 2026

Thanks @coderabbitai. Re the four review comments:

Fixed in 79eb9aa1 (test-only, no behavioral change to SVDSBTL):

  • src/Tests/CoolProp-Tests-SVDSBTLFailMap.cpp:7 — Updated header comment to list the three actual invocation tags ([if97_src], [heos_src_vs_if97], [heos_src_vs_heos]). Good catch — the example was stale.
  • src/Tests/CoolProp-Tests-SVDSBTLFailMap.cpp:147 — The CSV column was indeed inconsistent. Switched to write T_truth_refined so the column value and the dT_mK residual share the same reference frame.

Superseded by the stacked PR #2940 (TOMS748 replacement of the Newton fallback):

PR #2940 is stacked on this branch, so as soon as both merge the concerns vanish at the source.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 21, 2026

@ibell Thanks for the detailed write-up — all four concerns are fully accounted for:

  • 79eb9aa1: Both test-file fixes look clean. Aligning the CSV column to T_truth_refined removes the reference-frame ambiguity, and the corrected invocation-tag list keeps the file self-documenting.
  • PR #2940: The TOMS748 replacement is the right call. A bracketed root-find over [623.15, T_B23(p)] with an explicit ok flag is strictly safer than the seed-Newton approach — no silent fallthrough and a machine-epsilon convergence guarantee (or a reported failure). Happy to review that PR when it's ready.

I'll go ahead and resolve the four open review comments now.

🐇✨

✅ Actions performed

Comments resolved. Approval is disabled; enable reviews.request_changes_workflow to allow explicit top-level @coderabbitai resolve or @coderabbitai approve commands.

@ibell ibell merged commit 196b9fc into master May 21, 2026
33 of 34 checks passed
ibell added a commit that referenced this pull request May 21, 2026
Closes the remaining IF97 R1 conformance gap.  After foi.9.5 split
SUPER at p_B23(T) and dropped R3 worst-case from 188 K to 530 mK,
the worst R1 residuals (0.164% v at the compressed-liquid corner of
SUPER_R3) became visible.  Diagnosis: SUPER_R3 still spanned both
IF97 R1 (T<623.15) AND IF97 R3 (T>=623.15) territory.  At p ≈ 28 MPa
the R1 half occupied 63% of SUPER_R3's η axis while sharing the SVD
modes with the R3 half; the modes had to encode both smooth
compressed-liquid behavior and dense supercritical behavior, and R1
accuracy paid for it.

Fail-map evidence (PR #2938 / post-foi.9.5):
  R1 in subcritical LIQUID  (655 probes): T 0/655, v 1/655, s 9/655
  R1 in SUPER_R3            (251 probes): T 91/251, v 244/251, s 247/251
Same IF97 R1 physics, two orders of magnitude worse conformance
purely because SUPER_R3's SVD was shared with R3.

This commit adds a second SUPER atlas split, mirroring foi.9.5's
pattern:
1. build_h_R1R3_curve(heos, p_lo, p_hi) — samples h_IF97(623.15 K, p)
   at 64 log-spaced p knots over [pcrit, p_max], fits a
   region::CubicSplineCurve.
2. ph_subcritical (IF97 source path) replaces single SUPER_R3 with:
   - SUPER_R1_super: h ∈ [h_floor(p), h_R1R3(p))  — R1 territory
   - SUPER_R3_proper: h ∈ [h_R1R3(p), h_B23(p))   — R3 territory
   SUPER_R2 unchanged.  Region count for IF97 water surfaces now 5
   (LIQUID, VAPOR, SUPER_R1_super, SUPER_R3_proper, SUPER_R2) or 6
   with the SUPER_HIGH_P slab above 100 MPa.
3. Cache rev 7 → 8.

Measured impact (SVDSBTL&IF97 vs IF97, NT=200, NR=800, rank=20,
G13-15 IF97 budgets in parens):

                         foi.9.5 (B23 only)    + foi.9.9 (R1/R3)
  R1 T fails             91 / 906             0 / 906            (perm 25 mK)
  R1 T_max               134 mK               13 mK              (passes)
  R1 v fails             245 / 906            1 / 906            (perm 0.001%)
  R1 v_max               0.1640 %             0.0078 %           (21× better)
  R1 s fails             256 / 906            11 / 906
  R1 w fails             274 / 906            39 / 906

R1 conformance now matches LIQUID's residual floor (same compressed-
liquid physics, same fail counts).  R2 unchanged.  R3 essentially
unchanged — the structural 213% v outlier at T=663.7, p=26.6 (just
outside the default critical-patch HEOS-fallback bbox) remains; that
needs critical-patch widening (PR D / CoolProp-dxd) to absorb.

Refs: CoolProp-foi.9.9

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ibell added a commit that referenced this pull request May 21, 2026
Closes the remaining IF97 R1 conformance gap.  After foi.9.5 split
SUPER at p_B23(T) and dropped R3 worst-case from 188 K to 530 mK,
the worst R1 residuals (0.164% v at the compressed-liquid corner of
SUPER_R3) became visible.  Diagnosis: SUPER_R3 still spanned both
IF97 R1 (T<623.15) AND IF97 R3 (T>=623.15) territory.  At p ≈ 28 MPa
the R1 half occupied 63% of SUPER_R3's η axis while sharing the SVD
modes with the R3 half; the modes had to encode both smooth
compressed-liquid behavior and dense supercritical behavior, and R1
accuracy paid for it.

Fail-map evidence (PR #2938 / post-foi.9.5):
  R1 in subcritical LIQUID  (655 probes): T 0/655, v 1/655, s 9/655
  R1 in SUPER_R3            (251 probes): T 91/251, v 244/251, s 247/251
Same IF97 R1 physics, two orders of magnitude worse conformance
purely because SUPER_R3's SVD was shared with R3.

This commit adds a second SUPER atlas split, mirroring foi.9.5's
pattern:
1. build_h_R1R3_curve(heos, p_lo, p_hi) — samples h_IF97(623.15 K, p)
   at 64 log-spaced p knots over [pcrit, p_max], fits a
   region::CubicSplineCurve.
2. ph_subcritical (IF97 source path) replaces single SUPER_R3 with:
   - SUPER_R1_super: h ∈ [h_floor(p), h_R1R3(p))  — R1 territory
   - SUPER_R3_proper: h ∈ [h_R1R3(p), h_B23(p))   — R3 territory
   SUPER_R2 unchanged.  Region count for IF97 water surfaces now 5
   (LIQUID, VAPOR, SUPER_R1_super, SUPER_R3_proper, SUPER_R2) or 6
   with the SUPER_HIGH_P slab above 100 MPa.
3. Cache rev 7 → 8.

Measured impact (SVDSBTL&IF97 vs IF97, NT=200, NR=800, rank=20,
G13-15 IF97 budgets in parens):

                         foi.9.5 (B23 only)    + foi.9.9 (R1/R3)
  R1 T fails             91 / 906             0 / 906            (perm 25 mK)
  R1 T_max               134 mK               13 mK              (passes)
  R1 v fails             245 / 906            1 / 906            (perm 0.001%)
  R1 v_max               0.1640 %             0.0078 %           (21× better)
  R1 s fails             256 / 906            11 / 906
  R1 w fails             274 / 906            39 / 906

R1 conformance now matches LIQUID's residual floor (same compressed-
liquid physics, same fail counts).  R2 unchanged.  R3 essentially
unchanged — the structural 213% v outlier at T=663.7, p=26.6 (just
outside the default critical-patch HEOS-fallback bbox) remains; that
needs critical-patch widening (PR D / CoolProp-dxd) to absorb.

Refs: CoolProp-foi.9.9

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ibell added a commit that referenced this pull request May 21, 2026
Closes the remaining IF97 R1 conformance gap.  After foi.9.5 split
SUPER at p_B23(T) and dropped R3 worst-case from 188 K to 530 mK,
the worst R1 residuals (0.164% v at the compressed-liquid corner of
SUPER_R3) became visible.  Diagnosis: SUPER_R3 still spanned both
IF97 R1 (T<623.15) AND IF97 R3 (T>=623.15) territory.  At p ≈ 28 MPa
the R1 half occupied 63% of SUPER_R3's η axis while sharing the SVD
modes with the R3 half; the modes had to encode both smooth
compressed-liquid behavior and dense supercritical behavior, and R1
accuracy paid for it.

Fail-map evidence (PR #2938 / post-foi.9.5):
  R1 in subcritical LIQUID  (655 probes): T 0/655, v 1/655, s 9/655
  R1 in SUPER_R3            (251 probes): T 91/251, v 244/251, s 247/251
Same IF97 R1 physics, two orders of magnitude worse conformance
purely because SUPER_R3's SVD was shared with R3.

This commit adds a second SUPER atlas split, mirroring foi.9.5's
pattern:
1. build_h_R1R3_curve(heos, p_lo, p_hi) — samples h_IF97(623.15 K, p)
   at 64 log-spaced p knots over [pcrit, p_max], fits a
   region::CubicSplineCurve.
2. ph_subcritical (IF97 source path) replaces single SUPER_R3 with:
   - SUPER_R1_super: h ∈ [h_floor(p), h_R1R3(p))  — R1 territory
   - SUPER_R3_proper: h ∈ [h_R1R3(p), h_B23(p))   — R3 territory
   SUPER_R2 unchanged.  Region count for IF97 water surfaces now 5
   (LIQUID, VAPOR, SUPER_R1_super, SUPER_R3_proper, SUPER_R2) or 6
   with the SUPER_HIGH_P slab above 100 MPa.
3. Cache rev 7 → 8.

Measured impact (SVDSBTL&IF97 vs IF97, NT=200, NR=800, rank=20,
G13-15 IF97 budgets in parens):

                         foi.9.5 (B23 only)    + foi.9.9 (R1/R3)
  R1 T fails             91 / 906             0 / 906            (perm 25 mK)
  R1 T_max               134 mK               13 mK              (passes)
  R1 v fails             245 / 906            1 / 906            (perm 0.001%)
  R1 v_max               0.1640 %             0.0078 %           (21× better)
  R1 s fails             256 / 906            11 / 906
  R1 w fails             274 / 906            39 / 906

R1 conformance now matches LIQUID's residual floor (same compressed-
liquid physics, same fail counts).  R2 unchanged.  R3 essentially
unchanged — the structural 213% v outlier at T=663.7, p=26.6 (just
outside the default critical-patch HEOS-fallback bbox) remains; that
needs critical-patch widening (PR D / CoolProp-dxd) to absorb.

Refs: CoolProp-foi.9.9

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ibell added a commit that referenced this pull request May 21, 2026
Closes the remaining IF97 R1 conformance gap.  After foi.9.5 split
SUPER at p_B23(T) and dropped R3 worst-case from 188 K to 530 mK,
the worst R1 residuals (0.164% v at the compressed-liquid corner of
SUPER_R3) became visible.  Diagnosis: SUPER_R3 still spanned both
IF97 R1 (T<623.15) AND IF97 R3 (T>=623.15) territory.  At p ≈ 28 MPa
the R1 half occupied 63% of SUPER_R3's η axis while sharing the SVD
modes with the R3 half; the modes had to encode both smooth
compressed-liquid behavior and dense supercritical behavior, and R1
accuracy paid for it.

Fail-map evidence (PR #2938 / post-foi.9.5):
  R1 in subcritical LIQUID  (655 probes): T 0/655, v 1/655, s 9/655
  R1 in SUPER_R3            (251 probes): T 91/251, v 244/251, s 247/251
Same IF97 R1 physics, two orders of magnitude worse conformance
purely because SUPER_R3's SVD was shared with R3.

This commit adds a second SUPER atlas split, mirroring foi.9.5's
pattern:
1. build_h_R1R3_curve(heos, p_lo, p_hi) — samples h_IF97(623.15 K, p)
   at 64 log-spaced p knots over [pcrit, p_max], fits a
   region::CubicSplineCurve.
2. ph_subcritical (IF97 source path) replaces single SUPER_R3 with:
   - SUPER_R1_super: h ∈ [h_floor(p), h_R1R3(p))  — R1 territory
   - SUPER_R3_proper: h ∈ [h_R1R3(p), h_B23(p))   — R3 territory
   SUPER_R2 unchanged.  Region count for IF97 water surfaces now 5
   (LIQUID, VAPOR, SUPER_R1_super, SUPER_R3_proper, SUPER_R2) or 6
   with the SUPER_HIGH_P slab above 100 MPa.
3. Cache rev 7 → 8.

Measured impact (SVDSBTL&IF97 vs IF97, NT=200, NR=800, rank=20,
G13-15 IF97 budgets in parens):

                         foi.9.5 (B23 only)    + foi.9.9 (R1/R3)
  R1 T fails             91 / 906             0 / 906            (perm 25 mK)
  R1 T_max               134 mK               13 mK              (passes)
  R1 v fails             245 / 906            1 / 906            (perm 0.001%)
  R1 v_max               0.1640 %             0.0078 %           (21× better)
  R1 s fails             256 / 906            11 / 906
  R1 w fails             274 / 906            39 / 906

R1 conformance now matches LIQUID's residual floor (same compressed-
liquid physics, same fail counts).  R2 unchanged.  R3 essentially
unchanged — the structural 213% v outlier at T=663.7, p=26.6 (just
outside the default critical-patch HEOS-fallback bbox) remains; that
needs critical-patch widening (PR D / CoolProp-dxd) to absorb.

Refs: CoolProp-foi.9.9

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ibell added a commit that referenced this pull request May 21, 2026
…rted #2939 + #2940) (#2940)

* feat(svdsbtl): split SUPER_R3 at IF97 R1/R3 isotherm h_R1R3(p)

Closes the remaining IF97 R1 conformance gap.  After foi.9.5 split
SUPER at p_B23(T) and dropped R3 worst-case from 188 K to 530 mK,
the worst R1 residuals (0.164% v at the compressed-liquid corner of
SUPER_R3) became visible.  Diagnosis: SUPER_R3 still spanned both
IF97 R1 (T<623.15) AND IF97 R3 (T>=623.15) territory.  At p ≈ 28 MPa
the R1 half occupied 63% of SUPER_R3's η axis while sharing the SVD
modes with the R3 half; the modes had to encode both smooth
compressed-liquid behavior and dense supercritical behavior, and R1
accuracy paid for it.

Fail-map evidence (PR #2938 / post-foi.9.5):
  R1 in subcritical LIQUID  (655 probes): T 0/655, v 1/655, s 9/655
  R1 in SUPER_R3            (251 probes): T 91/251, v 244/251, s 247/251
Same IF97 R1 physics, two orders of magnitude worse conformance
purely because SUPER_R3's SVD was shared with R3.

This commit adds a second SUPER atlas split, mirroring foi.9.5's
pattern:
1. build_h_R1R3_curve(heos, p_lo, p_hi) — samples h_IF97(623.15 K, p)
   at 64 log-spaced p knots over [pcrit, p_max], fits a
   region::CubicSplineCurve.
2. ph_subcritical (IF97 source path) replaces single SUPER_R3 with:
   - SUPER_R1_super: h ∈ [h_floor(p), h_R1R3(p))  — R1 territory
   - SUPER_R3_proper: h ∈ [h_R1R3(p), h_B23(p))   — R3 territory
   SUPER_R2 unchanged.  Region count for IF97 water surfaces now 5
   (LIQUID, VAPOR, SUPER_R1_super, SUPER_R3_proper, SUPER_R2) or 6
   with the SUPER_HIGH_P slab above 100 MPa.
3. Cache rev 7 → 8.

Measured impact (SVDSBTL&IF97 vs IF97, NT=200, NR=800, rank=20,
G13-15 IF97 budgets in parens):

                         foi.9.5 (B23 only)    + foi.9.9 (R1/R3)
  R1 T fails             91 / 906             0 / 906            (perm 25 mK)
  R1 T_max               134 mK               13 mK              (passes)
  R1 v fails             245 / 906            1 / 906            (perm 0.001%)
  R1 v_max               0.1640 %             0.0078 %           (21× better)
  R1 s fails             256 / 906            11 / 906
  R1 w fails             274 / 906            39 / 906

R1 conformance now matches LIQUID's residual floor (same compressed-
liquid physics, same fail counts).  R2 unchanged.  R3 essentially
unchanged — the structural 213% v outlier at T=663.7, p=26.6 (just
outside the default critical-patch HEOS-fallback bbox) remains; that
needs critical-patch widening (PR D / CoolProp-dxd) to absorb.

Refs: CoolProp-foi.9.9

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(svdsbtl): replace Newton h-inversion with TOMS748 (foi.9.10)

The IF97-source update_state lambda in src/SBTL/SurfacePresets.cpp
Newton-iterated T to match h_target at each sample cell.  For cells in
IF97 R3 territory where HmassP_INPUTS throws (no published backward
equation), the Newton fell back to a fixed T=700 K initial guess.

Bug: at pressures where T_B23(p) < 700 K (i.e., R3 territory at
pcrit < p < ~28 MPa), the T=700 K seed lands in IF97 R2.  Newton then
crosses the R2/R3 boundary where cp values jump, overshoots into R1
(T < 623.15), bounces back into R2, etc. — fails to converge in 10
iterations.  The lambda returns with the iteration state stuck at
some wrong T, sample_grid records the stale T / s / ρ / w / η /
λ from that bad state, and the SVD encodes the corruption.

Direct probe of the worst-case cell (T_target=663.7 K, p=26.6 MPa):

  Forward IF97:           h=2170.28 kJ/kg, s=4516.80, ρ=321.62
  PR E Newton (broken):   T=728.65 (ΔT=+64.9 K)  h=2940.73 (Δh=+770.4!)
                          s=5642.70 (Δs=+1126 J/kgK)  ρ=116.92 (Δρ=−63.6%)

The "R3 v=213% / s=1283 J/kgK / T=81 K" outliers in the fail-map were
all this same sampling failure mode propagating through the SVD.  s
is smooth across the critical point, so a ~1.3 kJ/kgK s residual was
never plausibly an interpolation problem.

This commit replaces Newton with Boost's TOMS748 bracketed root-find
(already used in CoolProp via superancillary / FlashRoutines /
AbstractState — no new dependency):

1. solve_T_from_h_toms748() helper in the anonymous namespace.
   Lambda residual catches exceptions and returns NaN, so the
   sign-bracket check fails cleanly when the bracket touches an
   IF97 validity boundary.
2. Lambda flow:
   - HmassP_INPUTS for R1/R2/R5 (closed-form R7-97 backward seed).
   - Polish on [T_seed − 0.5, T_seed + 0.5] CLAMPED to IF97's
     [273.16, 2273.15] T validity, closing the ±25 mK R7-97 residual
     (critical for R2/R5 s conformance — cp·dT/T propagates 25 mK to
     ~0.25 J/(kg·K) s, over the 1e-3 perm budget).  If the clamped
     bracket doesn't span a sign change (near-T_min cells, or already-
     converged HmassP seed), the helper signals via *ok and we keep
     the HmassP state.
   - R3 fallback (HmassP throws): TOMS748 on [623.15, T_B23(p)].
3. Phase pin (PR E mechanism) preserved across the polish call so
   PT_INPUTS near the sat dome doesn't trip IF97's ε-band reject.
4. Cache rev 8 → 9.

Measured impact on SVDSBTL&IF97 vs IF97 fail-map (NT=200, NR=800,
rank=20, G13-15 IF97 budgets):

                          foi.9.9   foi.9.10    ratio
  R3 T fails              624       0           full pass
  R3 T max                81 K      19 mK       4300×
  R3 v max                213.1 %   0.035 %     6000×
  R3 s max                1283 J/kgK  0.38      3400×
  R3 w max                56.3 %    0.06 %      940×

R1 / R2 unchanged or marginally better (R1 s fails 11→7, R1 w fails
39→28).  R5 still clean.

The remaining R3 residuals (22 v fails / 26 s fails / 24 w fails, all
under ~0.06% / 0.4 J/kgK) are interpolation-residual-limited at the
near-critical sliver of SUPER_R3_proper — the standard SVD floor, no
longer pathological.

A first attempt removed the polish entirely to avoid clipping IF97's
T_min boundary; that regressed R2/R5 s conformance because the ±25 mK
R7-97 backward residual went uncorrected.  The clamped polish keeps
the R2/R5 s wins without the near-triple-point R1 regression.

Refs: CoolProp-foi.9.10

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(svdsbtl): TOMS748-polish patch state to forward-consistent T

The critical-patch HEOS-fallback routing calls
patch_source_ref_()->update(HmassP_INPUTS, h, p) which uses IF97's
published R7-97 backward equation (±25 mK from forward).  The SVD-
zone and sampling path now use forward-consistent T (foi.9.10), so
the patch-zone inherited the R7-97 floor while everywhere else got
machine precision — measurable as ~10-20 mK T residual and ~0.1-0.4
J/(kg·K) s residual on in-bbox probes.

Sanity check on the fail-map after the sampling-only TOMS748 fix:
26 fails sit inside the (T, p) Kunick box, all with |ΔT| ≤ 19.2 mK
— a textbook signature of the R7-97 backward floor (perm = 25 mK
for R1/R3, 10 mK for R2/R5).  Hypothesis confirmed.

Fix: after the patch's HmassP_INPUTS update, polish the state by
TOMS748-iterating on [T_seed−0.5, T_seed+0.5] (clamped to IF97's
[273.16, 2273.15] T validity) to bring T to forward consistency.
Mirrors the polish already applied in src/SBTL/SurfacePresets.cpp's
IF97 sampling lambda.  Wired into both HmassP_INPUTS and
HmolarP_INPUTS routing.

Measured impact on SVDSBTL&IF97 vs IF97 fail-map:

                          before     after      ratio
  R1 T max                12.9 mK    1.3 mK     10×
  R1 v fails              1          0          full pass
  R1 v max                0.0078%    0.0003%    26×
  R2 T max                17.8 mK    0.7 mK     25×
  R2 v fails              3          0          full pass
  R2 s fails              7          4
  R3 T max                19.2 mK    0.25 mK    76×
  R3 v fails              22         0          full pass
  R3 v max                0.0346%    0.0002%    173×
  R3 s max                0.38 J/kgK 0.0016     235×
  R3 w max                0.061%     0.0003%    200×

Zero T fails, zero v fails everywhere.  R5 still 100% clean.  10 s
residuals (max 0.015 J/kgK in R1) and 32 w residuals (max 0.006%
in R1) remain — all under 6× perm, all structural SVD interpolation
residual that's no longer dominated by sampling-side artifacts.

For HEOS source backend (patch source = HEOS), the polish is a no-op
in convergence (HEOS HmassP_INPUTS is already iterative internally
and forward-consistent), at the cost of one extra HEOS evaluation
per in-bbox query.  Negligible.

Refs: CoolProp-foi.9.10

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(svdsbtl): use std::uintmax_t for TOMS748 iteration counter (MSVC compat)

`boost::uintmax_t` requires `<boost/cstdint.hpp>` on MSVC, which the
Tauri-GUI build didn't pull in transitively.  `std::uintmax_t` is the
same type (boost typedefs it from `<cstdint>` in modern releases) and
works in every toolchain.  Closes the Windows build failure on PR
#2940.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(svdsbtl): CodeRabbit findings on polish + R3 fallback paths

Three actionable findings from CodeRabbit on PR #2940:

**SVDSBTLBackend.cpp polish_patch_state_**

* Replaced the hardcoded `kT_min = 273.16` / `kT_max = 2273.15` (water /
  IAPWS-95-specific) with `s.Tmin()` / `s.Tmax()` so the polish stays
  in-range for non-water patch_source_ backends.  patch_source_ can be
  HEOS / REFPROP / IF97 at different fluids, each with its own
  envelope; pinning to water bounds either silently clipped (HEOS
  Tmin > 273.16, e.g. Ammonia 195.495 K) or, more dangerously,
  overshot (any backend with Tmax < 2273.15, which is most of them).
* Wrapped `toms748_solve` + the final `s.update(PT_INPUTS, ...)` in
  try/catch that restores T_seed via PT_INPUTS on any exception, so a
  solver throw or final-update failure leaves `s` at the backward seed
  (±25 mK floor) instead of an undefined half-mutated state at the
  last failed PT probe.

**SurfacePresets.cpp ph_subcritical IF97 sampling lambda**

* R3 fallback (line ~380): guard `if97_T_B23_K(p)` with the curve's
  validity range [16.529, 100] MPa.  Outside that p range the closed-
  form inverse returns garbage (sub-623.15 K at low p, NaN from the
  negative-sqrt at high p), and the previous code would have committed
  that garbage T as a real state via `s.update(PT_INPUTS, p, T_R3)`.
  The R3 fallback shouldn't be reached outside [16.529, 100] MPa in
  practice — that's the SUPER_HIGH_P slab where ph_subcritical builds
  a single region without a B23 split — but the guard makes the
  failure mode explicit (throw → SurfaceFactory NaN-fills the cell)
  rather than silent corruption.
* R3 fallback: pass `&ok` to `solve_T_from_h_toms748` and propagate
  failure by throwing if the bracket misses, so the cell becomes a
  NaN fill instead of pinning an endpoint as a real PT state.  The
  helper already advertises this contract via the `ok` parameter; the
  caller just wasn't using it.  Same pattern as the existing polish
  branch above.
* Polish branch (line ~395): when `solve_T_from_h_toms748` returns
  `ok = false`, restore `s` via `s.update(HmassP_INPUTS, h, p)`.  The
  solver mutates `s` via its bracket-residual probes; on a miss the
  last probe leaves `s` at PT_INPUTS(p, T_lo or T_hi) instead of the
  backward HmassP seed established at line 362.  Without the restore,
  downstream `read_property` calls would have read off the near-
  endpoint PT state — which can still be a valid IF97 read but
  silently violates the lambda's contract of "if polish misses, fall
  through to the HmassP backward seed (±25 mK floor)".

Test impact: 43 SVDSBTL test cases / 671 assertions still pass; no
behaviour change in successful-polish or successful-R3-fallback paths
(the common case).  Only edge / failure modes differ: cleaner failure
propagation, no garbage-T commits, water-bound clipping removed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ibell added a commit that referenced this pull request May 23, 2026
…s (CoolProp-4no.1)

Review-comment fixes (Web/coolprop/SVDSBTL.rst):

* Subcritical-LIQUID region's lower boundary is the melting curve at
  high p (with EOS-domain low-T isotherm fallback elsewhere), not a
  pure isotherm.  Code path: SurfacePresets.cpp:215-219 "non-isothermal
  floor that walks T up if T_min < T_melt(p) at high p".

* Native C++ `update + rhomass + T` is region-dependent — fast_evaluate
  per-probe measured at SUPER 228 ns, LIQUID 191 ns, SUPERHEAT 208 ns
  vs TWO-PHASE 1582 ns.  Updated regime 1 prose to call this out:
  well under 200 ns/call in single-phase / supercritical, ~1-1.5 µs/call
  in two-phase where SuperAncillary or PQ flash runs per probe.

* Dropped the explicit recommendation that the update-and-accessor
  path is "what most user code should look like" — fast_evaluate is
  meaningfully faster for any throughput workload, and the doc should
  point users there.

* REFPROP PQ flash cost claim corrected.  The flash itself is
  ~1.5 µs/call native (REFPROP::Water PQ(1MPa, 0.5) measured at
  1.807 µs Python, ~1.5 µs native).  The SVDSBTL&REFPROP fallback
  wrapper (lever rule + endpoint conversions on top of the flash)
  brings the user-visible per-probe cost to ~3-4 µs (SVDSBTL&REFPROP
  HmassP in-dome measured at 3.658 µs Python).  My previous wording
  ("REFPROP's PQ flash ~3-5 µs") conflated the two; separated in the
  new prose.  CoolProp-077 motivation re-framed accordingly.

Cross-links from related doc pages (bd CoolProp-4no.1):

* Web/index.rst — added SVDSBTL bullet to the top-level "What is
  CoolProp?" feature list.
* Web/coolprop/Tabular.rst — `.. seealso::` block right after the
  BICUBIC/TTSE backend intro, recommending SVDSBTL for new
  throughput-oriented code.
* Web/coolprop/HighLevelAPI.rst — backend-selector example block
  expanded to mention SVDSBTL&IF97 / SVDSBTL&HEOS with the
  ALLOW_SVDSBTL_IN_PROPSSI gate caveat.
* Web/coolprop/LowLevelAPI.rst — AbstractState-instance-count warning
  extended to call out SVDSBTL's ~80-140 ms table-load cost.
* Web/coolprop/REFPROP.rst — `.. seealso::` block recommending
  SVDSBTL&REFPROP for throughput workloads against REFPROP truth.
* Web/coolprop/changelog.rst — Unreleased highlights entry for the
  SVDSBTL backend landing (PRs #2917, #2938-#2940, #2944-#2957).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ibell added a commit that referenced this pull request May 23, 2026
…2959)

* docs(svdsbtl): correct 19 doc bugs uncovered by audit (CoolProp-fhp)

Doc-only PR — no behavior change.  Audit via five parallel verification
subagents (perf / API / critical-patch / architecture / accuracy)
found 9 outright errors, 4 near-right inaccuracies, and 6 imprecisions
in Web/coolprop/SVDSBTL.rst.  Highlights:

- Removed `set_critical_bbox_multipliers(...)` C++ entry point claim;
  the function does not exist in include/, src/, or wrappers/.
- Removed `ALTERNATIVE_SVDTABLES_DIRECTORY` config-key claim; not
  registered in include/Configuration.h, not consulted in
  SVDSurfaceSerializer.cpp.  Tracked via bd CoolProp-fhp for actual
  implementation.
- Removed XDG_DATA_HOME-on-Linux claim; get_home_dir() at
  src/CPfilepaths.cpp:157 only reads $HOME.
- Clarified `registered_input_pairs()` is C++-only (not Python-bound).

- Native C++ `update+rhomass+T` updated 200 ns → ~1 µs (dominated
  by update(); accessors <10 ns).  Cython overhead ~0.3 µs.  Total
  Python-visible cost ~1.3 µs/iter.
- fast_evaluate example switched to supercritical-only probe
  distribution (`p>p_c, h>h_c`) so the per-probe number reflects
  the locate+basis cost isolated from the critical patch.  Updated
  to 4 outputs to match the prose, perf claim tightened to
  ~300-330 ns/probe, marginal cost 57 ns → ~35 ns/output.
- Table-load cost generalised 80 ms → 80-140 ms (typical 7 MB
  table vs the 14 MB SVDSBTL&IF97 HmassP table).
- R245fa cold build under N=4 threads: 65→25 s claim corrected to
  measured 122→54 s on Apple Silicon; ratio (~2.25×) holds.
- Per-fluid critical-patch bbox table updated from cached
  multipliers on the current master.  Water/HEOS (p,h) bbox h-range
  corrected 1.55-2.93 → 1.74-2.65 MJ/kg.

- IF97 G13-15 conformance prose tightened: only T(p,h) is
  uniformly conformant in R1/R2/R5; v is conformant only in R5;
  w in R2 and R5; s exceeds budget in all four regions.  Transport
  property miss broadened from "in R3" to "across R1, R2, R3".
- REFPROP PQ flash fallback cost corrected ~ms → ~3-5 µs (off by
  three orders of magnitude); CoolProp-077 motivation re-framed
  around cross-DLL hop + thread-safety, not headline latency.

- Architecture imprecisions tightened: grid orientation 200×800
  relabeled N_h×N_p (200 on secondary, 800 on primary); per-table
  sample count clarified (160k per region × 3-7 regions); subcritical
  boundary curves clarified (one is sat dome, other is isotherm —
  not both dome); C¹ continuity sharpened to C² with default
  natural-cubic-spline slopes.

Also fixes a stale comment block in
include/CoolProp/Backends/SVDSBTL/SVDSBTLBackend.h near
auto_calibrate_critical_bbox_() (calibration budgets and initial
bbox / iteration count both wrong relative to .cpp constants).

Verified by superpowers:code-reviewer subagent against master;
no blocking findings.  Numbers cross-checked against on-disk
~/.CoolProp/SVDTables/*.critpatch.bin cache values, the rendered
Web/fluid_properties/IF97_conformance.rst.in fail-map, and live
Python runtime measurements.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(svdsbtl): address review comments + cross-link from related pages (CoolProp-4no.1)

Review-comment fixes (Web/coolprop/SVDSBTL.rst):

* Subcritical-LIQUID region's lower boundary is the melting curve at
  high p (with EOS-domain low-T isotherm fallback elsewhere), not a
  pure isotherm.  Code path: SurfacePresets.cpp:215-219 "non-isothermal
  floor that walks T up if T_min < T_melt(p) at high p".

* Native C++ `update + rhomass + T` is region-dependent — fast_evaluate
  per-probe measured at SUPER 228 ns, LIQUID 191 ns, SUPERHEAT 208 ns
  vs TWO-PHASE 1582 ns.  Updated regime 1 prose to call this out:
  well under 200 ns/call in single-phase / supercritical, ~1-1.5 µs/call
  in two-phase where SuperAncillary or PQ flash runs per probe.

* Dropped the explicit recommendation that the update-and-accessor
  path is "what most user code should look like" — fast_evaluate is
  meaningfully faster for any throughput workload, and the doc should
  point users there.

* REFPROP PQ flash cost claim corrected.  The flash itself is
  ~1.5 µs/call native (REFPROP::Water PQ(1MPa, 0.5) measured at
  1.807 µs Python, ~1.5 µs native).  The SVDSBTL&REFPROP fallback
  wrapper (lever rule + endpoint conversions on top of the flash)
  brings the user-visible per-probe cost to ~3-4 µs (SVDSBTL&REFPROP
  HmassP in-dome measured at 3.658 µs Python).  My previous wording
  ("REFPROP's PQ flash ~3-5 µs") conflated the two; separated in the
  new prose.  CoolProp-077 motivation re-framed accordingly.

Cross-links from related doc pages (bd CoolProp-4no.1):

* Web/index.rst — added SVDSBTL bullet to the top-level "What is
  CoolProp?" feature list.
* Web/coolprop/Tabular.rst — `.. seealso::` block right after the
  BICUBIC/TTSE backend intro, recommending SVDSBTL for new
  throughput-oriented code.
* Web/coolprop/HighLevelAPI.rst — backend-selector example block
  expanded to mention SVDSBTL&IF97 / SVDSBTL&HEOS with the
  ALLOW_SVDSBTL_IN_PROPSSI gate caveat.
* Web/coolprop/LowLevelAPI.rst — AbstractState-instance-count warning
  extended to call out SVDSBTL's ~80-140 ms table-load cost.
* Web/coolprop/REFPROP.rst — `.. seealso::` block recommending
  SVDSBTL&REFPROP for throughput workloads against REFPROP truth.
* Web/coolprop/changelog.rst — Unreleased highlights entry for the
  SVDSBTL backend landing (PRs #2917, #2938-#2940, #2944-#2957).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(highlevelapi): drop tabular-backend mentions from high-level page

Per review comment: the high-level page (PropsSI) should not point
users at tabular backends.  PropsSI constructs an AbstractState on
every call, which is the catastrophic case for any tabular backend
(every call re-pays the table-load cost) — exactly what the SVDSBTL
page warns against in its "PropsSI is off by default" gate.

Removed:
* The "If speed is an issue, you can look into table-based
  interpolation methods using TTSE or bicubic interpolation" sentence
  (pre-existing) from the PropsSI section.
* The "BICUBIC / TTSE / SVDSBTL" backend-selector paragraph that I
  added in a33ede4.

The IF97 mention (single-formulation, no per-call table load) stays
since it doesn't have the same anti-pattern.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(highlevelapi): restore the pre-existing TTSE/bicubic sentence

Overshot in 2b8a6b7: the "remove tabular-backend info" instruction
was scoped to my added paragraph (the BICUBIC/TTSE/SVDSBTL backend-
selector example), not the pre-existing "If speed is an issue, you
can look into ... TTSE or bicubic interpolation" sentence further up.
Restored that sentence verbatim.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(highlevelapi): point speed-tip at SVDSBTL instead of TTSE/bicubic

Updated the "If speed is an issue" sentence in the PropsSI section
to reference the SVDSBTL backend rather than the older TTSE/bicubic
methods.  SVDSBTL is the recommended throughput-oriented tabular
backend for new code (per the seealso in Tabular.rst added in
a33ede4); pointing to it from the high-level page keeps the
recommendation consistent across pages.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@henningjp
Copy link
Copy Markdown
Contributor

Curious why you would create your own b23 functions when they could be called directly from IF97::Region23_T(p) and IF97::Region23_p(T).

@ibell
Copy link
Copy Markdown
Contributor Author

ibell commented May 25, 2026

@henningjp it is a good question. @claude thought it was a good idea, but I agree it should use the existing implementation. Also their implementation was incorrect the first time :)

ibell added a commit that referenced this pull request May 25, 2026
…cal copy (#2992)

Per henningjp's review on #2938: the SBTL IF97 R2/R3-split path carried
its own closed-form inverse of the B23 boundary (if97_T_B23_K with a
local copy of the three B23 coefficients).  That's a duplicate of the
if97 library's IF97::Region23_p(p) — same constants, same
θ = b + √((p_MPa − c)/a) — so delete the wrapper and call
IF97::Region23_p directly at both sites (build_h_B23_curve and the R3
sampling fallback).  IF97::Region23_p is now the single source of
truth for the boundary.

CoolProp builds the if97 dep in Pa units (p_fact = 1e6, same as the
IF97::psat97 / Tsat97 calls in HumidAirProp), so p passes through in
Pa with no conversion.

Behavior-preserving (formula was byte-identical); SVDSBTL&IF97 water
G13-15 conformance tests pass unchanged under preflight.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ibell ibell added this to the v8.0.0 milestone May 27, 2026
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