Fix recuperator effectiveness via pinch-point analysis#65
Conversation
The effectiveness values from twine-models are incorrect for variable-property fluids — the per-segment C_min approach diverges as segment count increases (twine-models#73). Add src/effectiveness.rs: a standalone module that computes correct effectiveness by bisecting on the cold-side outlet temperature using RecuperatorGivenOutlet to find q_max where min_delta_t reaches zero. Effectiveness is then q_actual / q_max. The facades (simple and recomp) now use this instead of the twine-models values. The cycle solvers are untouched — the fix is entirely in how the facade reports effectiveness.
gdtroszak
left a comment
gdtroszak
left a comment
There was a problem hiding this comment.
Clean fix. The pinch-point bisection is the right approach, and the separation from the solvers keeps the change well-scoped. A few minor notes inline.
| Thermo: EffectivenessThermo<Fluid>, | ||
| { | ||
| let q_actual_w = q_actual.get::<watt>(); | ||
| if q_actual_w == 0.0 { |
There was a problem hiding this comment.
Exact float comparison — if q_actual picks up a tiny negative from solver noise, this falls through to bisection and could return a nonsensical result. q_actual_w <= 0.0 (or < ε) would be safer.
There was a problem hiding this comment.
Good catch — changed to <= 0.0 so tiny negatives from solver noise get caught. (384d51d)
| bisection::solve_from_bracket(&recup, &problem, bracket, &config, observer).ok()?; | ||
|
|
||
| let q_max_w = solution.snapshot.output.q_dot.magnitude().get::<watt>(); | ||
| if q_max_w == 0.0 { |
There was a problem hiding this comment.
Same concern here. If bisection converges to a degenerate case where q_max is near-zero but not exactly zero, q_actual / tiny clamps to 1.0. Probably fine in practice (q_max ≈ 0 implies q_actual ≈ 0), but a comment explaining why this is safe would help.
There was a problem hiding this comment.
Agreed. Changed to <= 0.0 for consistency with the first guard, and added a comment explaining why near-zero is safe: q_max ≈ 0 implies q_actual ≈ 0, and .clamp(0.0, 1.0) handles any residual overshoot. (384d51d)
| config.hx.recuperator.segments, | ||
| thermo, | ||
| ) | ||
| .unwrap_or(f64::NAN); |
There was a problem hiding this comment.
Silent NaN in a result struct is worth a deliberate decision. Downstream consumers (especially WASM/JS) may not handle NaN well — JSON serializes it as null in some libraries, others choke.
Options:
- Document on the output struct fields that effectiveness can be NaN
- Return an error from the facade if the computation fails
- Fall back to the (known-incorrect) twine-models value with a flag
Not blocking, but worth deciding explicitly.
There was a problem hiding this comment.
Changed all effectiveness fields to Option<f64> — on both simple and recomp output structs. None is explicit, serializes cleanly to JSON null, and doesn't require downstream consumers to handle NaN. (384d51d)
…veness - Use <= 0.0 instead of == 0.0 for q_actual and q_max guards to handle solver noise producing tiny negatives. - Add comment explaining why near-zero q_max is safe. - Change effectiveness fields from f64 to Option<f64> on both simple and recomp output structs. None is explicit and JSON-safe, unlike NaN.
Remove Effectiveness type and fields from solution structs. twine-models 0.4 drops built-in effectiveness from recuperator outputs; brayton already computes it independently via pinch-point analysis (added in #65).
Remove Effectiveness type and fields from solution structs. twine-models 0.4 drops built-in effectiveness from recuperator outputs; brayton already computes it independently via pinch-point analysis (added in #65).
2 participants
The recuperator effectiveness values from twine-models are incorrect for variable-property fluids like supercritical CO₂. The underlying computation sums per-segment
C_min * ΔTto getq_max, but picks the minimum-capacitance stream independently per segment — producing a denominator that doesn't correspond to any physically realizable heat transfer. The approximation diverges rather than converges as segment count increases. See twine-models#73.This PR adds a standalone
effectivenessmodule that computes correct effectiveness via pinch-point analysis: bisect on the cold-side outlet temperature usingRecuperatorGivenOutletto findq_maxwheremin_delta_treaches zero, theneffectiveness = q_actual / q_max. The facades use this instead of the twine-models values. The cycle solvers are untouched.