Bump twine to 0.6 and use solve_from_bracket#71
Conversation
36fd7d2 to
c1e0c6c
Compare
| GivenUaError::EqualInletTemperatures => Self::Convergence { | ||
| message: "equal inlet temperatures: positive UA requires a temperature difference" | ||
| .to_owned(), | ||
| iterations: None, |
There was a problem hiding this comment.
EqualInletTemperatures maps to Convergence, but it's a precondition violation like NegativeUa, not a convergence failure. Was this intentional to avoid a breaking change to RecuperatorGivenUaError, or worth a dedicated variant?
There was a problem hiding this comment.
Good catch — wasn't intentional, just avoiding the variant addition without thinking it through. Added a dedicated EqualInletTemperatures variant to RecuperatorGivenUaError and mapped directly to it. It's a precondition violation like NegativeUa, not a convergence failure.
Also corrected the doc comments — the original framing claimed no heat transfer is possible with equal inlet temperatures, but that's only true for perfect gases. With real fluids and pressure drop (Joule-Thomson effect), temperature gradients can develop along the exchanger even from equal inlets. The actual constraint is that the bisection bracket collapses to zero width.
Commit: cfdeaf0
| // Any second-law violation during midpoint iteration is a | ||
| // genuine overshoot — the candidate outlet temperature | ||
| // exceeds physical limits, so UA is overestimated. | ||
| if matches!(event, bisection::Event::ModelFailed { .. }) { |
There was a problem hiding this comment.
The old handler specifically matched SolveError::SecondLawViolation. This catches any ModelFailed — if the inner solve can fail for other reasons (thermo backend error, etc.), those now silently become assume_positive instead of propagating. Is that intentional?
There was a problem hiding this comment.
Not intentional — the old code matched specifically and I lost that precision in the refactor. Now matches only SolveError::SecondLawViolation { .. } inside the ModelFailed event. Other model errors (thermo backend failures, etc.) return None from the observer and propagate as solver failures.
Commit: cfdeaf0
c1e0c6c to
beea6b5
Compare
Bump twine-core, twine-solvers, and twine-observers to 0.6. Refactor the given_ua solver to build the bracket from known physics rather than evaluating endpoints. At T_out = T_top_in, UA is zero (negative residual); at T_out = T_bottom_in, UA is infinite (positive residual). This eliminates both INLET_PROXIMITY_THRESHOLD_K and ENTHALPY_RELATIVE_ZERO — hardcoded tolerances that papered over edge cases the caller should control. The observer now only sees midpoints via solve_from_bracket, where any second-law violation is a genuine overshoot.
beea6b5 to
cfdeaf0
Compare
Bump twine dependencies to 0.6 and take advantage of the new
solve_from_bracketAPI to simplify the discretized UA solver.Two hardcoded tolerance constants are removed:
INLET_PROXIMITY_THRESHOLD_K— distinguished FP noise from real second-law violations near the bracket boundaryENTHALPY_RELATIVE_ZERO— snapped tiny enthalpy differences to zero in the forward solverBoth were workarounds for edge cases at
T_out == T_in. Withsolve_from_bracket, the bracket signs are known from physics — no endpoint evaluation needed — and the observer only sees midpoints where these edge cases don't arise.