feat: #1140 - Implement mass_transfer_rate() and rate() with tests#1147
feat: #1140 - Implement mass_transfer_rate() and rate() with tests#1147Gorkowski merged 3 commits intouncscode:mainfrom
Conversation
Add vapor-pressure surface helper to compute Kelvin-corrected partial pressures for latent-heat condensation. Implement non-isothermal mass_transfer_rate and rate to sanitize non-finite deltas, include latent heat/thermal conductivity corrections, and preserve isothermal parity when latent heat is absent. Expand CondensationLatentHeat tests with parity, shape, skip-partitioning, concentration scaling, and non-finite-sanitization coverage. Update E5-F3 planning docs to reflect current status. Closes uncscode#1140 ADW-ID: 2476d14e
Mark E5 and E5-F3-P2 status updates in dev plans and indices, refresh docs/readme to reflect latent-heat mass-transfer and rate coverage, and expand latent-heat strategy docstrings for clarity. Closes uncscode#1140 ADW-ID: 2476d14e
ADW Code ReviewPR: #1147 - feat: #1140 - Implement mass_transfer_rate() and rate() with tests Summary
Critical Issues (Must Fix)None. Warnings (Should Fix)
Suggestions (Overview)
Positive Observations
Inline CommentsDetailed feedback has been posted as inline comments on the following locations:
This review was generated by ADW Multi-Agent Code Review System. |
|
WARNING: Strategy API inconsistency in
Suggested fix: # Option A: add to base interface
class CondensationStrategy(ABC):
def rate(self, ..., dynamic_viscosity: float | NDArray[np.float64]) -> ...:
...
# Option B: keep signature consistent and pass via state objectKeeping signatures aligned prevents LSP violations and avoids runtime errors when swapping strategies. |
|
WARNING: Missing non-finite validation in latent-heat path
Suggested fix: validate_inputs({
"latent_heat": "finite",
"thermal_conductivity": "finite",
"vapor_pressure_surface": "finite",
})This keeps the latent-heat path consistent with other input validation and avoids silent NaN propagation. |
|
WARNING: The Kelvin term can yield non-finite values, which then flow into Suggested fix: vapor_pressure_surface = np.asarray(vapor_pressure_surface)
if not np.all(np.isfinite(vapor_pressure_surface)):
raise ValueError("vapor_pressure_surface must be finite")Failing fast makes invalid inputs visible and prevents downstream NaNs. |
|
WARNING:
Suggested fix: if not np.all(np.isfinite(pressure_delta)):
raise ValueError("pressure_delta contains non-finite values")Failing loudly here will surface invalid upstream inputs instead of silently masking them. |
There was a problem hiding this comment.
Pull request overview
Implements the non-isothermal (latent-heat-corrected) condensation rate path in CondensationLatentHeat, keeping isothermal parity when no latent heat strategy is configured, and adds targeted tests + doc updates to reflect the feature’s completion status.
Changes:
- Added
_get_vapor_pressure_surface()and implementedCondensationLatentHeat.mass_transfer_rate()with an optional latent-heat correction branch (and non-finite Δp sanitization). - Implemented
CondensationLatentHeat.rate()to scale by raw particle concentration and apply skip-partitioning, mirroring isothermal behavior. - Expanded condensation strategy tests for parity, latent-heat reduction, shape handling, skip-partitioning, scaling, and sanitization; refreshed docs to reflect E5 progress.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
particula/dynamics/condensation/condensation_strategies.py |
Implements latent-heat corrected mass-transfer/rate flow and adds a helper to obtain surface vapor pressure. |
particula/dynamics/condensation/tests/condensation_strategies_test.py |
Adds fixtures and tests covering parity, reduction, shapes, scaling, skip-partitioning, and sanitization. |
readme.md |
Updates feature bullets to reflect that latent-heat mass_transfer_rate()/rate() are implemented. |
docs/index.md |
Updates landing-page feature text to reflect latent-heat strategy completion status. |
adw-docs/dev-plans/features/E5-F3-condensation-latent-heat-strategy.md |
Marks E5-F3-P2 complete and adjusts planning metadata. |
adw-docs/dev-plans/epics/index.md |
Updates E5 epic status to “In Progress”. |
adw-docs/dev-plans/epics/E5-non-isothermal-condensation.md |
Updates E5 epic metadata and phase checklist status. |
adw-docs/dev-plans/README.md |
Updates E5 epic status and removes outdated P1 reference. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1840,24 +1946,50 @@ def rate( | |||
| pressure: float, | |||
| dynamic_viscosity: Optional[float] = None, | |||
There was a problem hiding this comment.
CondensationLatentHeat.rate() adds a dynamic_viscosity parameter, but CondensationStrategy.rate() (and the other condensation strategies) do not accept this argument. This breaks API parity with CondensationIsothermal.rate() and can cause override/type-checking issues. Consider removing dynamic_viscosity from rate() (keep it only on mass_transfer_rate()), or, if viscosity needs to be configurable at the rate() level, update the base ABC + all strategy implementations consistently.
🤖 Agent Developer Workflow
Current Status
Workflow Plan
📋 Recent Activity🔍 Archived UpdatesThis comment is automatically updated as the workflow progresses |
Harden latent-heat condensation by validating pressure deltas, latent heat, thermal conductivity, and surface vapor pressure. Extract Kelvin/partial-pressure helper to avoid duplication and add bounds checks for skip-partitioning indices. Update docs and expand tests for error paths and index validation. Closes uncscode#1147 ADW-ID: 98ecae9c
Fix SummaryAddressed PR review feedback on condensation strategy API parity, latent-heat validation, pressure-delta handling, skip-partitioning bounds, and documentation/testing gaps in the condensation latent-heat path. Changes Made
Review Comments Addressed
Files Changed
Automated fix by ADW workflow |
Target Branch:
mainFixes #1140 | Workflow:
2476d14eSummary
Adds the non-isothermal condensation path to
CondensationLatentHeat, reusing the isothermal flow through transport/pressure steps and branching to a latent-heat correction that uses surface vapor pressure and thermal conductivity. Implementsrate()parity with the isothermal strategy and tightens handling of non-finite pressure deltas. Expands latent-heat tests to cover parity, reduction, shapes, scaling, skip-partitioning, and sanitization.What Changed
New Components
_get_vapor_pressure_surfacehelper (incondensation_strategies.py) to extract surface equilibrium vapor pressure for the latent-heat correction without changing the public API.Modified Components
particula/dynamics/condensation/condensation_strategies.py— implementedCondensationLatentHeat.mass_transfer_rate()with latent-heat branch usingget_mass_transfer_rate_latent_heat+get_thermal_conductivity, reusing isothermal steps 1–3 and sanitizing pressure deltas; implementedrate()to scale by raw concentration and apply skip-partitioning; kept isothermal parity when no latent heat strategy is provided.particula/dynamics/condensation/tests/condensation_strategies_test.py— added full latent-heat fixtures, parity/reduction/shape/skip-partitioning/scaling/sanitization tests, and updated the legacy stub test to only assertstep()is still unimplemented.adw-docs/dev-plans/*,docs/index.md,readme.md) — refreshed feature plan and index references for E5-F3 P2 completion.Tests Added/Updated
particula/dynamics/condensation/tests/condensation_strategies_test.py— coverage for isothermal parity, latent-heat reduction, single- vs multi-species shapes, rate scaling and skip-partitioning, surface vapor-pressure helper, and non-finite pressure-delta sanitization.How It Works
Implementation Notes
pressure_deltavalues are zeroed to avoid propagating NaNs/±inf into rates._get_vapor_pressure_surfacemirrors the front half ofcalculate_pressure_delta(), squeezing(n, 1)activity output to 1D to maintain shape parity.diffusion_coefficientand thermal conductivity at the call temperature; fallback path remains bit-for-bit withCondensationIsothermalwhen no latent heat strategy is present.rate()multiplies by raw concentration (legacy-compatible) and respects configured skip-partitioning indices.Testing
pytest particula/dynamics/condensation/tests/condensation_strategies_test.py -k "LatentHeat"pytest(workflow test step)