feat(process): OutletFluidNotAchievableError and SpeedSolver EOS failure handling#1543
feat(process): OutletFluidNotAchievableError and SpeedSolver EOS failure handling#1543
Conversation
| ) | ||
| minimum_speed_configuration = SpeedConfiguration(speed=minimum_speed_within_capacity) | ||
| minimum_speed_outlet_stream = func(minimum_speed_configuration) | ||
| except OutletFluidNotAchievableError: |
There was a problem hiding this comment.
this method is getting long, would it make sense to split it into (reusable) steps/phases - to easier understand/read each step?
| assert outlet_stream.pressure_bara == expected_pressure | ||
|
|
||
|
|
||
| class FluidNotAchievableProcessUnit(ProcessUnit): |
There was a problem hiding this comment.
is this like a "probe" to test the stream?
There was a problem hiding this comment.
does it need to be a processunit? can we rather mock the propagate_stream method on a DummyProcessUnit with "no" logic, since what we are testing is that the "framework" around can handle the error state correctly?
6bce7ae to
ba42051
Compare
ba42051 to
5df72d6
Compare
…inarySearchResult
…ssorThermodynamicCalculationError
5df72d6 to
754df3e
Compare
kjbrak
left a comment
There was a problem hiding this comment.
Please see the inline comments.
| return Solution( | ||
| success=False, | ||
| configuration=configuration, | ||
| failure_event=OutletFluidNotAchievableEvent( |
There was a problem hiding this comment.
This new failure event still needs a caller-side short-circuit. OutletPressureSolver.find_solution() currently treats every failed speed solution as a pressure-control case: it applies the returned speed, runs anti-surge, and then re-runs the pipeline. For an OUTLET_FLUID_NOT_ACHIEVABLE result, that speed is specifically not runnable, so the typed solver failure can become an escaping OutletFluidNotAchievableError again. Consider returning this failure directly from OutletPressureSolver before anti-surge/pressure-control handling, preserving speed_solution.failure_event and the speed configuration.
| if isinstance(result, Solution): | ||
| return result | ||
| max_speed_configuration = result | ||
| maximum_speed_outlet_stream = get_outlet_stream(speed=result.speed) |
There was a problem hiding this comment.
When max speed fails EOS, this block adjusts max_speed_configuration to the highest EOS-feasible speed and computes maximum_speed_outlet_stream at that adjusted speed. If the target is still above that adjusted maximum, the failure branch below currently returns the original boundary max, which makes the returned configuration inconsistent with achievable_value and can cause callers to re-run the EOS-invalid speed. Consider returning max_speed_configuration from that target-not-achievable branch instead.
| try: | ||
| get_outlet_stream(x) | ||
| return ACCEPT_AND_GO_HIGHER | ||
| except (OutletFluidNotAchievableError, RateTooHighError, RateTooLowError): |
There was a problem hiding this comment.
This search can still leak a capacity error when the lower end of the speed range is invalid for capacity while the upper end is invalid for EOS. For example, boundary.min may raise RateTooHighError, an intermediate speed may be valid, and high speeds may raise OutletFluidNotAchievableError; the pre-check only catches EOS failures, and treating RateTooHighError as "go lower" moves in the wrong direction for speed-dependent max-flow capacity. Consider first narrowing to the lowest capacity-feasible speed, then searching for the highest EOS-feasible speed within that narrowed interval.
Type of Work
Have you remembered and considered?
docs/drafts/next.draft.md)What is this PR all about?
SpeedSolverhad no handling for EOS/PHflash failures inside the compressor. Ifcalculate_outlet_pressure_and_stream()raised — due to NaN properties, non-convergence, or a Java exception from NeqSim near the dense/supercritical boundary — the exception escaped the solver uncaught. Theexcept EcalcErrorguard inCompressor.propagate_stream()also missed Java exceptions, which surface asPy4JJavaError/JPype exceptions rather thanEcalcErrorsubclasses.This PR introduces
OutletFluidNotAchievableError(a typedProcessError) raised byCompressor.propagate_stream()on any exception from the flash layer, and wires it intoSpeedSolverat all three call sites (max-speed probe, min-speed probe, root-finding loop). When the boundary probe fails, the solver binary-searches for the highest/lowest EOS-valid speed before proceeding. A pre-check against the opposite boundary prevents the search from exhausting iterations when no speed in the range produces a valid outlet — which would otherwise raiseDidNotConvergeErrorunhandled.What else did you consider?
Catching only
EcalcErrorat the flash boundary — but Java exceptions from NeqSim are notEcalcErrorsubclasses, so this left the original bug in place for the cases that actually occur in production.Handling the recovery inside
Compressorrather than in the solver — but the solver already owns the speed-search logic and is the right place to decide what to do with a failed operating point.Between the lines?
The min-speed upward search mirrors the max-speed downward search and uses the same
BinarySearchStrategysemantics as the existingRateTooHighErrorrecovery. The pre-checks are structurally necessary:BinarySearchStrategy.search()converges tomax(x)whereaccepted=True, and if no point is ever accepted it exhaustsmax_iterationswithout a valid result.