feat: #1139 - [E5-F3-P1] Create CondensationLatentHeat class skeleton with tests#1146
Conversation
Add CondensationLatentHeat with resolver logic and stubs to support future latent-heat condensation behavior. Extend condensation tests and exports to cover the new strategy and its resolution warnings. Closes uncscode#1139 ADW-ID: 72bc5e5e
Mark E5-F3 P1 in progress across dev plan docs and update public docs/readme to mention the CondensationLatentHeat scaffold and strategy resolution behavior. Expand the CondensationLatentHeat docstrings to describe inputs and stubbed method expectations. Closes uncscode#1139 ADW-ID: 72bc5e5e
ADW Code ReviewPR: #1146 - feat: #1139 - [E5-F3-P1] Create CondensationLatentHeat class skeleton 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: latent_heat validation should reject non-finite / decide array support
Suggested fix: if not np.isfinite(latent_heat):
raise ValueError("latent_heat must be finite")Decide whether array-like values are supported; if not, validate and raise to avoid silent behavior. |
|
WARNING: Align rate/step signatures with CondensationStrategy The Suggested fix: def rate(self, aerosol, gas, temperature, pressure, time_step):
...Keeping signatures identical preserves strategy interchangeability and typing. |
|
WARNING: Docstring needs types/units + latent_heat constraints The class docstring lists attributes without types/units and Suggested fix: latent_heat: float # J/kg, finite, non-negativeClarify scalar/array expectations and fallback behavior in the docstrings. |
There was a problem hiding this comment.
Pull request overview
Adds an initial CondensationLatentHeat condensation strategy scaffold to support latent-heat strategy/constant resolution and diagnostics, while deferring the non-isothermal mass-transfer implementation to later phases. The PR also wires the new symbol into the public dynamics/condensation namespaces and updates docs/dev-plan indices accordingly.
Changes:
- Add
CondensationLatentHeatskeleton with latent-heat strategy resolution andlast_latent_heat_energydiagnostic. - Export
CondensationLatentHeatviaparticula.dynamicsandparticula.dynamics.condensation, with smoke tests for exports. - Add unit tests covering instantiation paths, warning logs, and NotImplemented stubs; refresh docs/dev-plan references.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
particula/dynamics/condensation/condensation_strategies.py |
Adds CondensationLatentHeat class skeleton with latent-heat strategy resolution + stubs. |
particula/dynamics/condensation/__init__.py |
Re-exports CondensationLatentHeat from the condensation subpackage. |
particula/dynamics/__init__.py |
Re-exports CondensationLatentHeat from the top-level dynamics namespace. |
particula/dynamics/condensation/tests/condensation_strategies_test.py |
Adds TestCondensationLatentHeat for resolution priority, logging, diagnostics, and stubs. |
particula/dynamics/tests/condensation_exports_test.py |
Extends export smoke tests to include CondensationLatentHeat. |
readme.md |
Documents the new condensation strategy scaffold in the feature list. |
docs/index.md |
Updates homepage bullet to mention CondensationLatentHeat scaffold. |
adw-docs/dev-plans/features/index.md |
Marks E5-F3 status as “In Progress”. |
adw-docs/dev-plans/features/E5-F3-condensation-latent-heat-strategy.md |
Updates status, dates, and phase checklist details for #1139. |
adw-docs/dev-plans/epics/E5-non-isothermal-condensation.md |
Notes #1139 in the epic’s phase listing and updates “Last Updated”. |
adw-docs/dev-plans/README.md |
Updates dev-plan README to reflect E5-F3 in progress (P1, #1139). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def rate( | ||
| self, | ||
| particle: ParticleRepresentation | ParticleData, | ||
| gas_species: GasSpecies | GasData, | ||
| temperature: float, | ||
| pressure: float, | ||
| dynamic_viscosity: Optional[float] = None, | ||
| ) -> NDArray[np.float64]: | ||
| """Return the condensation rate (stub). | ||
|
|
||
| Args: | ||
| particle: Particle representation providing radius and activity | ||
| information. | ||
| gas_species: Gas species supplying vapor properties and | ||
| concentrations. | ||
| temperature: System temperature in Kelvin. | ||
| pressure: System pressure in Pascals. | ||
| dynamic_viscosity: Optional dynamic viscosity override. | ||
|
|
||
| Returns: | ||
| Condensation rate per particle and per species in kg/s. | ||
|
|
||
| Raises: | ||
| NotImplementedError: Implemented in E5-F3-P2/P3. | ||
| """ | ||
| raise NotImplementedError("Implemented in E5-F3-P2/P3") | ||
|
|
||
| def step( | ||
| self, | ||
| particle: ParticleRepresentation | ParticleData, | ||
| gas_species: GasSpecies | GasData, | ||
| temperature: float, | ||
| pressure: float, | ||
| time_step: float, | ||
| dynamic_viscosity: Optional[float] = None, | ||
| ) -> ( | ||
| Tuple[ParticleRepresentation, GasSpecies] | Tuple[ParticleData, GasData] | ||
| ): |
There was a problem hiding this comment.
rate() and step() add a dynamic_viscosity parameter that is not part of the CondensationStrategy interface (and is not present on CondensationIsothermal / CondensationIsothermalStaggered). For API consistency across condensation strategies, either remove dynamic_viscosity from these method signatures or update the base ABC + other implementations in the same change set.
| if latent_heat_array.shape == (): | ||
| latent_heat_value = float(latent_heat_array) | ||
| if latent_heat_value > 0: | ||
| return ConstantLatentHeat(latent_heat_value) |
There was a problem hiding this comment.
Use a keyword argument when constructing ConstantLatentHeat (e.g., latent_heat_ref=...) to match how this codebase instantiates it elsewhere and to make the units/meaning of the positional value unambiguous.
| return ConstantLatentHeat(latent_heat_value) | |
| return ConstantLatentHeat(latent_heat_ref=latent_heat_value) |
| cond = CondensationLatentHeat(molar_mass=0.018, latent_heat=2.26e6) | ||
|
|
||
| self.assertIsInstance(cond._latent_heat_strategy, ConstantLatentHeat) | ||
| assert cond._latent_heat_strategy is not None |
There was a problem hiding this comment.
Avoid mixing bare assert with unittest.TestCase assertions. Using self.assertIsNotNone(...) keeps failure reporting consistent and avoids python -O potentially skipping the check.
| assert cond._latent_heat_strategy is not None | |
| self.assertIsNotNone(cond._latent_heat_strategy) |
🤖 Agent Developer Workflow
Current Status
Workflow Plan
📋 Recent Activity🔍 Archived UpdatesThis comment is automatically updated as the workflow progresses |
Reject array-like or non-finite latent heat inputs and use keyworded constant latent heat construction to clarify units. Align CondensationLatentHeat rate/step signatures with the base strategy and update tests to assert ValueError handling. Update E5-F3 planning docs to reflect validation expectations. Closes uncscode#1146 ADW-ID: 197d81b2
Fix SummaryImplemented the fixes outlined in the workflow plan for PR #1146, including aligning Changes Made
Review Comments Addressed
(Review comment IDs not recorded in workflow state.) Files Changed
Automated fix by ADW workflow |
Target Branch:
mainFixes #1139 | Workflow:
72bc5e5eSummary
Adds the
CondensationLatentHeatstrategy skeleton with latent-heat resolutionlogic and diagnostics, enabling strategy/constant/isothermal fallbacks while
deferring non-isothermal mass-transfer implementation. Exposes the new strategy
through the condensation and dynamics namespaces, and extends tests to cover
instantiation, warnings, and stub behavior.
What Changed
New Components
particula/dynamics/condensation/condensation_strategies.py- AddedCondensationLatentHeatwith latent-heat strategy resolution, diagnostics,and stubbed abstract methods for later phases.
Modified Components
particula/dynamics/condensation/__init__.py- ExportedCondensationLatentHeatvia the condensation package.particula/dynamics/__init__.py- Re-exportedCondensationLatentHeatin thetop-level dynamics namespace.
particula/dynamics/condensation/tests/condensation_strategies_test.py-Added
TestCondensationLatentHeatcoverage for resolution priority,warnings, and stubbed method behavior.
particula/dynamics/tests/condensation_exports_test.py- Verified exportsacross
particula.dynamicsandparticula.dynamics.condensation.adw-docs/dev-plans/**,docs/index.md,readme.md- Refreshed plan anddocumentation indices.
How It Works
CondensationLatentHeatmirrors the isothermal initialization path, thenresolves latent heat in priority order:
LatentHeatStrategywhen provided.ConstantLatentHeat.isothermal behavior (
None).The core mass-transfer methods are stubbed with
NotImplementedErrorto keepinstantiation and diagnostics testable ahead of P2–P3 implementation.
Implementation Notes
condensation strategies while introducing a clear, testable latent-heat
resolution policy.
to avoid CI
-Werrorfailures.Testing
pytest(workflow Run Tests phase)pytest particula/dynamics/condensation/tests/condensation_strategies_test.py -vpytest particula/dynamics/tests/condensation_exports_test.py -v