Skip to content

feat: #1141 - [E5-F3-P3] Implement step() for particle-resolved with energy tracking and tests#1148

Merged
Gorkowski merged 3 commits intouncscode:mainfrom
Gorkowski:issue-1141-adw-2260fbb3
Mar 5, 2026
Merged

feat: #1141 - [E5-F3-P3] Implement step() for particle-resolved with energy tracking and tests#1148
Gorkowski merged 3 commits intouncscode:mainfrom
Gorkowski:issue-1141-adw-2260fbb3

Conversation

@Gorkowski
Copy link
Copy Markdown
Collaborator

Target Branch: main

Fixes #1141 | Workflow: 2260fbb3

Summary

Implements the latent-heat-aware step() for CondensationLatentHeat, mirroring the isothermal workflow while recording per-step latent heat release. Adds a dedicated energy update hook, forwards optional dynamic_viscosity through the step, and keeps API compatibility with MassCondensation. Documentation now describes the latent-heat step behavior and energy diagnostic.

What Changed

New Components

  • (None)

Modified Components

  • particula/dynamics/condensation/condensation_strategies.py - Implemented CondensationLatentHeat.step() with latent-heat energy tracking, added overloads including dynamic_viscosity, and shared helper _update_latent_heat_energy plus proper skip-partitioning/shape handling.
  • particula/dynamics/condensation/tests/condensation_strategies_test.py - Expanded TestCondensationLatentHeat with step mass/energy conservation, sign, type, gas-update, viscosity pass-through, and MassCondensation compatibility coverage.
  • docs/Features/condensation_strategy_system.md and indexes/readme - Documented latent-heat step support and energy diagnostic.
  • adw-docs/dev-plans/* and docs/index.md/readme.md - Synced plan references and feature links for this phase.

Tests Added/Updated

  • condensation_strategies_test.py – New cases for mass conservation (legacy & data paths), energy matches dm × L, sign conventions, isothermal parity & zero-energy fallback, return types, gas update behavior, dynamic viscosity forwarding, and MassCondensation runnable integration.

How It Works

mass_transfer_rate(dynamic_viscosity?)
        │
        ▼
 get_mass_transfer (inventory + Δt)
        │
        ▼
_update_latent_heat_energy(dm, L)
        │
        ▼
update particle masses
        │
        ▼
update gas (optional)

The step unwraps particle/gas data, enforces single-box, computes per-particle mass transfer with skip-partitioning, records latent heat via get_latent_heat_energy_released, then updates particle masses and optionally gas concentration. Energy is overwritten each call and reflects condensation (>0) or evaporation (<0).

Implementation Notes

  • dynamic_viscosity now threads through step() to mass_transfer_rate for transport overrides.
  • _update_latent_heat_energy centralizes the diagnostic and zeros it when no latent heat strategy is configured (isothermal parity).
  • Shape guards mirror isothermal behavior to keep multi-species/legacy paths consistent and ensure MassCondensation continues to work without API changes.

Testing

  • Unit tests: Extensive TestCondensationLatentHeat coverage for mass/energy conservation, sign conventions, parity with isothermal, type returns, gas updates, viscosity forwarding, and runnable compatibility.
  • Manual verification: Not required (unit coverage is targeted and deterministic).

Mirror CondensationIsothermal.step for latent-heat strategies, update
particle and gas states, and record per-step latent-heat energy with
last_latent_heat_energy. Forward dynamic_viscosity to mass_transfer_rate
and add overloads for legacy and data inputs.

Add latent-heat step tests covering mass conservation, energy tracking,
sign conventions, isothermal parity, gas updates, dynamic viscosity
pass-through, and MassCondensation compatibility.

Closes uncscode#1141

ADW-ID: 2260fbb3
Update feature docs, plans, and README to reflect the implemented
CondensationLatentHeat step, energy diagnostics, and viscosity
override notes. Expand the strategy system guide with latent heat
details and example usage.

Closes uncscode#1141

ADW-ID: 2260fbb3
Copilot AI review requested due to automatic review settings March 4, 2026 14:01
@Gorkowski Gorkowski added agent Created or managed by ADW automation blocked Blocked - review required before ADW can process labels Mar 4, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR completes the particle-resolved CondensationLatentHeat.step() workflow by mirroring the existing isothermal step() logic while adding a per-step latent heat energy diagnostic and threading an optional dynamic_viscosity override through the step path. It also expands unit tests and updates user/dev documentation to reflect the new capability.

Changes:

  • Implement CondensationLatentHeat.step() (legacy + data-only) with latent heat energy tracking and dynamic_viscosity pass-through.
  • Add/extend tests validating mass conservation, energy tracking/sign conventions, isothermal parity, type behavior, gas updates, and runnable compatibility.
  • Update feature docs/readme and dev-plan docs to reflect that latent-heat step support is now implemented.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
particula/dynamics/condensation/condensation_strategies.py Implements CondensationLatentHeat.step(), adds _update_latent_heat_energy, overloads, and forwards dynamic_viscosity.
particula/dynamics/condensation/tests/condensation_strategies_test.py Adds comprehensive CondensationLatentHeat.step() coverage (mass/energy/parity/types/gas update/viscosity/runnable).
docs/Features/condensation_strategy_system.md Documents latent-heat step behavior and the new energy diagnostic.
docs/index.md Updates top-level feature list to reflect implemented latent-heat step().
readme.md Updates feature bullets to reflect implemented latent-heat step() and energy diagnostic.
adw-docs/dev-plans/features/index.md Updates plan status text for E5-F3.
adw-docs/dev-plans/features/E5-F3-condensation-latent-heat-strategy.md Marks E5-F3-P3 complete and links to issue #1141; adds changelog entry.
adw-docs/dev-plans/epics/E5-non-isothermal-condensation.md Marks E5-F3-P3 complete and updates epic changelog.
adw-docs/dev-plans/README.md Updates dev-plan README to reference P3/#1141.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +145 to +169
### CondensationLatentHeat (energy diagnostics)

`CondensationLatentHeat` mirrors the isothermal step but applies a latent-heat
correction when a latent heat strategy (or scalar fallback) is provided. It
records `last_latent_heat_energy` each step (positive for condensation,
negative for evaporation) and accepts a `dynamic_viscosity` override for
particle-resolved workflows.

```python
latent = par.dynamics.CondensationLatentHeat(
molar_mass=0.018,
diffusion_coefficient=2e-5,
accommodation_coefficient=1.0,
latent_heat=2.4e6, # J/kg fallback
)
particle, gas = latent.step(
particle=particle,
gas_species=gas,
temperature=298.15,
pressure=101325.0,
time_step=1.0,
dynamic_viscosity=1.8e-5,
)
energy_released = latent.last_latent_heat_energy
```
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs/example use last_latent_heat_energy without stating units. Given the condensation workflow uses mass concentrations (kg/m^3), this diagnostic is currently an energy density (J/m^3) unless multiplied by the parcel/box volume. Please clarify the units in this section so users don’t interpret it as total energy [J].

Copilot uses AI. Check for mistakes.
Comment on lines +2152 to +2156

self._update_latent_heat_energy(mass_transfer, temperature)

species_mass = particle_data.masses[0]
if species_mass.ndim == 1:
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_update_latent_heat_energy() is called before mass_transfer is reshaped/clipped/broadcast to match particle_data.masses[0]. In cases where get_mass_transfer() returns a different species width than the particle inventory (handled a few lines later), the diagnostic can include mass-transfer components that are never applied to the particle/gas updates. Consider moving the energy update to after the mass-transfer shape normalization (or passing the normalized mass_transfer).

Copilot uses AI. Check for mistakes.
Comment on lines +2045 to +2059
"""Store latent heat energy released for the current step.

Args:
mass_transfer: Per-particle mass transfer in kg for this step.
temperature: System temperature in Kelvin.
"""
if self._latent_heat_strategy is None:
self.last_latent_heat_energy = 0.0
return

latent_heat = self._latent_heat_strategy.latent_heat(temperature)
self.last_latent_heat_energy = float(
np.sum(
get_latent_heat_energy_released(
mass_transfer,
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstrings describe mass_transfer as kg and last_latent_heat_energy as [J], but in step() the mass transfer returned by get_mass_transfer() is scaled by particle concentration (#/m^3), matching the existing isothermal step’s kg/m^3 semantics. That makes last_latent_heat_energy an energy density (J/m^3) unless it’s multiplied by the box volume. Please either update the docs/attribute text to reflect J/m^3, or multiply by particle_data.volume[0] if the intended diagnostic is total energy [J].

Copilot uses AI. Check for mistakes.
@Gorkowski
Copy link
Copy Markdown
Collaborator Author

ADW Code Review

PR: #1148 - feat: #1141 - [E5-F3-P3] Implement step() for particle-resolved with energy tracking and tests
Review Date: 2026-03-04
Reviewers: Code Quality, Correctness, Performance (C++/Python), Security


Summary

Severity Count
Critical 0
Warning 10
Suggestion 6

Warnings (Should Fix)

  • condensation_strategies.py: energy computed before normalization.
  • condensation_strategies.py: non-finite pressure_delta parity with isothermal path.
  • condensation_strategies.py: missing validation on volume/concentration (norm_conc).
  • condensation_strategies.py: gas concentration can go negative during update.
  • condensation_strategies.py: extra allocations in mass clamp path.
  • condensation_strategies.py: missing update_gases=False test for energy tracking.
  • condensation_strategies.py: strategy interface inconsistency.
  • docs: statuses out of sync (epic/feature).
  • docs: energy units likely J/m^3; clarify.
  • tests: add parity coverage for non-finite pressure delta handling.

Suggestions (Overview)

  • Refactor step duplication to reduce divergence risk.
  • Avoid duplicate mask compute for normalization.
  • Use np.atleast_1d for mass_rate.
  • Add data-only energy tracking test.
  • Add skip-partitioning energy test.
  • Consider moving last_latent_heat_energy out of strategy state.

Positive Observations

  • Energy tracking is integrated with particle-resolved step flow.
  • Tests introduced for new energy tracking pathways.

Inline Comments

Detailed feedback has been posted as inline comments on the following locations:

  • condensation_strategies.py:~2153 - normalize before latent heat energy update
  • condensation_strategies.py:~1955–1962 - sanitize non-finite pressure delta
  • condensation_strategies.py:(norm_conc) - validate volume/concentration
  • condensation_strategies.py:(gas update) - clamp gas concentration to nonnegative
  • condensation_strategies.py:~2145–2156 - reduce allocations in mass clamp

This review was generated by ADW Multi-Agent Code Review System.
Reviewers: Quality, Correctness, C++ Performance, Python Performance, Security

@Gorkowski
Copy link
Copy Markdown
Collaborator Author

WARNING: Latent heat energy computed before normalization

_update_latent_heat_energy is called before the mass transfer normalization, so energy can be computed against pre-normalized values.

Suggested fix:

# normalize mass_transfer first, then compute latent heat energy
mass_transfer = _normalize_mass_transfer(...)
self._update_latent_heat_energy(mass_transfer, ...)

This keeps energy tracking consistent with the normalized mass transfer actually applied.

@Gorkowski
Copy link
Copy Markdown
Collaborator Author

WARNING: Sanitize non-finite pressure_delta for parity with isothermal

The isothermal path handles non-finite pressure_delta, but this block does not, leading to divergent behavior.

Suggested fix:

pressure_delta = np.where(np.isfinite(pressure_delta), pressure_delta, 0.0)

Aligns behavior across pathways and prevents NaNs from propagating.

@Gorkowski
Copy link
Copy Markdown
Collaborator Author

WARNING: Validate volume and concentration in norm_conc

norm_conc relies on volume and concentration being finite/positive but does not validate inputs.

Suggested fix:

if not np.all(np.isfinite(volume)) or np.any(volume <= 0):
    raise ValueError("volume must be positive and finite")
if not np.all(np.isfinite(concentration)):
    raise ValueError("concentration must be finite")

Prevents invalid normalization and downstream NaNs.

@Gorkowski
Copy link
Copy Markdown
Collaborator Author

WARNING: Clamp gas concentration to nonnegative

Gas concentration updates can drive values below zero when mass transfer is large.

Suggested fix:

gas_concentration = np.maximum(gas_concentration, 0.0)

Avoids negative concentrations and keeps physical invariants intact.

@Gorkowski
Copy link
Copy Markdown
Collaborator Author

SUGGESTION: Reduce allocations in mass clamp

This section allocates multiple temporary arrays during clamp.

Suggested fix:

# in-place add + maximum where possible
np.add(mass_rate, 0.0, out=mass_rate)
np.maximum(mass_rate, 0.0, out=mass_rate)

This reduces temporary allocations in a hot path.

@Gorkowski Gorkowski added request:fix Request AI to implement review suggestions adw:in-progress ADW workflow actively processing - remove to re-trigger and removed blocked Blocked - review required before ADW can process labels Mar 5, 2026
@Gorkowski
Copy link
Copy Markdown
Collaborator Author

Gorkowski commented Mar 5, 2026

🤖 Agent Developer Workflow

  • Description: Direct PR fix workflow triggered by request:fix label. Analyzes actionable review comments, generates fix plan, implements changes, validates, polishes, tests, formats, and ships fixes without creating intermediate GitHub issues.
  • ADW ID: 31535132
  • Current Phase: Workflow completed successfully
  • Current Step: Analyze actionable PR review comments and generate implementation plan
  • Started: 2026-03-05T01:39:11.925488Z
  • Last Update: 2026-03-05T02:13:13.282841Z

Current Status

  • Workflow completed in 33m 59s

Workflow Plan

  • ✅ Analyze PR Comments (at 2026-03-05T01:39:55.776759Z)
  • ✅ Build Implementation (at 2026-03-05T01:48:00.045840Z)
  • ✅ Validate Implementation and Fix Gaps (at 2026-03-05T01:50:49.760619Z)
  • ✅ Polish Code (at 2026-03-05T01:51:05.538903Z)
  • ✅ Run Tests (at 2026-03-05T02:11:40.510987Z)
  • ✅ Format Code and Add Docstrings (at 2026-03-05T02:12:26.172764Z)
  • ✅ Ship Fix (at 2026-03-05T02:13:11.638035Z)
📋 Recent Activity
[2026-03-05T01:50:49.760642Z] Completed step 3/7: Validate Implementation and Fix Gaps
[2026-03-05T01:50:51.380200Z] Starting step 4/7: Polish Code
[2026-03-05T01:51:05.538924Z] Completed step 4/7: Polish Code
[2026-03-05T01:51:07.223651Z] Starting step 5/7: Run Tests
[2026-03-05T02:11:40.511008Z] Completed step 5/7: Run Tests
[2026-03-05T02:11:42.301515Z] Starting step 6/7: Format Code and Add Docstrings
[2026-03-05T02:12:26.172783Z] Completed step 6/7: Format Code and Add Docstrings
[2026-03-05T02:12:27.884868Z] Starting step 7/7: Ship Fix
[2026-03-05T02:13:11.638058Z] Completed step 7/7: Ship Fix
[2026-03-05T02:13:13.281270Z] Workflow fix finished: 7/7 steps completed
🔍 Archived Updates
[2026-03-05T01:39:11.990547Z] Workflow fix started with 7 steps
[2026-03-05T01:39:13.933533Z] Starting step 1/7: Analyze PR Comments
[2026-03-05T01:39:55.776780Z] Completed step 1/7: Analyze PR Comments
[2026-03-05T01:39:57.402381Z] Starting step 2/7: Build Implementation
[2026-03-05T01:48:00.045863Z] Completed step 2/7: Build Implementation
[2026-03-05T01:48:01.679970Z] Starting step 3/7: Validate Implementation and Fix Gaps

This comment is automatically updated as the workflow progresses

Successfully fixed:
- Refactored CondensationLatentHeat.step into helpers to reduce complexity and keep behavior intact.
- Sanitized non-finite pressure_delta values to mirror isothermal behavior.
- Validated volume/concentration inputs for norm_conc and raise clear errors.
- Normalized mass_transfer shape before latent heat energy accounting.
- Clamped gas concentration updates to nonnegative values.
- Documented latent heat energy units and expanded latent heat step tests.

Remaining issues: none.

Closes uncscode#1148

ADW-ID: 31535132
@Gorkowski
Copy link
Copy Markdown
Collaborator Author

Fix Summary

Implemented the requested latent-heat step fixes and documentation updates, with expanded test coverage for the new behavior. Key changes include normalized mass-transfer energy tracking, non-finite pressure-delta handling parity with the isothermal path, input validation for normalization, gas concentration clamping, and allocation-reducing in-place updates.

Changes Made

  • Ensured latent-heat energy updates use normalized mass transfer so diagnostics reflect applied transfer.
  • Sanitized non-finite pressure_delta values to match isothermal behavior.
  • Added validation for volume/concentration inputs used in normalization and enforced nonnegative gas concentration updates.
  • Reduced allocations in mass-clamp paths via in-place operations and minor low-risk refactors (e.g., avoiding duplicate mask compute, np.atleast_1d for mass_rate).
  • Clarified latent-heat energy unit semantics in docs and docstrings.
  • Expanded tests for update_gases=False, non-finite pressure_delta parity, data-only energy tracking, and skip-partitioning energy behavior.

Review Comments Addressed

  • Clarified last_latent_heat_energy units in docs and docstrings.
  • Fixed latent-heat energy update to use normalized mass transfer (matches applied transfer).
  • Added parity handling for non-finite pressure_delta and validation/clamping improvements.
  • Added missing tests for latent-heat energy tracking variants and update_gases behavior.

(Comment IDs were not recorded; summary is based on the workflow spec.)

Files Changed

  • docs/Features/condensation_strategy_system.md
  • docs/index.md
  • particula/dynamics/condensation/condensation_strategies.py
  • particula/dynamics/condensation/tests/condensation_strategies_test.py
  • readme.md

Automated fix by ADW workflow 31535132

@Gorkowski Gorkowski merged commit 3c8b036 into uncscode:main Mar 5, 2026
7 checks passed
@Gorkowski Gorkowski deleted the issue-1141-adw-2260fbb3 branch March 5, 2026 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adw:in-progress ADW workflow actively processing - remove to re-trigger agent Created or managed by ADW automation request:fix Request AI to implement review suggestions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[E5-F3-P3] Implement step() for particle-resolved with energy tracking and tests

2 participants