Release: 0.5.0 - #9
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
✅ Files skipped from review due to trivial changes (7)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughReleases v0.5.0: version/compatibility bumps and changelog updates; docs build now extracts Pluto notebook titles; adds a rain-radar Pluto notebook with observables/LUTs and plots; and adds an optional radiative-transfer example (manifest, README, shared helpers, fake-WRF generator, and a WRF→dual-pol radar CLI). ChangesTransitionMatrices v0.5.0 Release with Examples
Sequence Diagram(s)sequenceDiagram
participant User
participant Notebook as rain_radar.jl
participant Scattering as Scattering Table
participant DSD as Drop Size Distribution
participant Moments as Dual-Pol Moments
User->>Notebook: Define water/ice dielectrics
Notebook->>Scattering: Compute per-diameter spheroid table
User->>Notebook: Request radar observables at rain rate
Notebook->>DSD: Marshall-Palmer drop distribution
DSD->>Moments: Weight amplitudes by DSD
Moments->>Notebook: Return ZH, ZDR, ρhv, KDP
User->>Notebook: Build multi-frequency TB LUT
Notebook->>Scattering: Generate table per frequency
Scattering->>Moments: Integrate TB per rain rate
Moments->>Notebook: Return LUT rows
Notebook->>User: Render observable plots
sequenceDiagram
participant CLI
participant Generator as generate_fake_wrf.jl
participant Processor as wrf_dualpol_radar.jl
participant WRFFile as WRF NetCDF
participant ScatterCache as Scattering Cache
participant GridLoop as Threaded Grid
participant Output as Output NetCDF
CLI->>Generator: --nx --ny --nz --nt
Generator->>Generator: Synthetic fields (lat, lon, time, rain, vapor)
Generator->>WRFFile: Write NetCDF variables
CLI->>Processor: --input --dsd --freq --cache
Processor->>ScatterCache: Load or compute scattering table
Processor->>WRFFile: Read WRF fields
Processor->>GridLoop: Iterate cells in parallel
GridLoop->>GridLoop: Compute density, DSD, radar moments
GridLoop->>Output: Write ZH, ZV, ZDR, RHOHV, KDP
Processor->>CLI: Print summary (valid cells, output path)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9 +/- ##
=======================================
Coverage 95.63% 95.63%
=======================================
Files 32 32
Lines 4403 4403
=======================================
Hits 4211 4211
Misses 192 192
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
examples/rain_radar.jl (1)
42-65: ⚡ Quick winPrefer the
c0_m_per_s()helper for consistency.Line 62 hardcodes
2.99792458e8while line 80 (inice_refractive_index) correctly callsc0_m_per_s(). Using the helper improves maintainability.♻️ Proposed fix
εimag = (εs - ε∞) * x * cospi(α / 2) / denominator + - σ * λ_m / (2π * 2.99792458e8 * ε0) + σ * λ_m / (2π * c0_m_per_s() * ε0)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/rain_radar.jl` around lines 42 - 65, In water_refractive_index replace the hardcoded speed-of-light literal (2.99792458e8) used in the εimag term with the existing c0_m_per_s() helper for consistency with ice_refractive_index; specifically update the σ * λ_m / (2π * 2.99792458e8 * ε0) expression inside water_refractive_index to use c0_m_per_s() so the computation reads σ * λ_m / (2π * c0_m_per_s() * ε0), keeping all other math unchanged.examples/radiative_transfer/generate_fake_wrf.jl (1)
5-8: ⚖️ Poor tradeoffConsider extracting shared constants and functions.
The constants
R_DRY_AIR,WATER_DENSITY,MASS_COEFF, andRAIN_N0(lines 5-8), theair_densityfunction (lines 56-60), and thewrite_varhelper (lines 116-121) are duplicated inwrf_dualpol_radar.jl. If these values or implementations diverge, the generator and radar pipeline could become inconsistent.Consider extracting shared constants and utilities into a common
physics.jlorshared.jlfile that both scripts can include.Also applies to: 56-60, 116-121
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/radiative_transfer/generate_fake_wrf.jl` around lines 5 - 8, The constants R_DRY_AIR, RAIN_N0, WATER_DENSITY, MASS_COEFF and the helper functions air_density and write_var are duplicated between generate_fake_wrf.jl and wrf_dualpol_radar.jl; extract them into a new shared module (e.g., physics or shared) to avoid drift, move the constant definitions and the implementations of air_density and write_var into that module, export those symbols, and update both scripts to import the module and reference R_DRY_AIR, RAIN_N0, WATER_DENSITY, MASS_COEFF, air_density, and write_var instead of redefining them.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/radiative_transfer/wrf_dualpol_radar.jl`:
- Around line 134-150: The cache validation in load_or_compute_scattering_table
only compares Ds and freq_hz; include the IITM solver parameters (e.g., nmax,
nr, ntheta supplied via the solver argument) in both the payload and the
equality check so cached tables are invalidated when solver options change: when
loading, compare payload.Ds, payload.freq_hz and payload.solver_params (or
explicit payload.nmax/payload.nr/payload.ntheta) against the current Ds, freq_hz
and solver settings; when writing, serialize the table together with those
solver parameter values so future loads can detect mismatches; update references
to scattering_table and serialize/deserialize payload shape accordingly.
In `@examples/rain_radar.jl`:
- Line 137: The line assigning Cext_mm2 contains a stray closing parenthesis;
update the call to calc_Cext so it uses the correct syntax and parameters
(calc_Cext(T, λ)) by removing the extra ')' — ensure you call the exported
function calc_Cext (alias of extinction_cross_section in TransitionMatrices)
with the variables T and λ and assign the result to Cext_mm2.
---
Nitpick comments:
In `@examples/radiative_transfer/generate_fake_wrf.jl`:
- Around line 5-8: The constants R_DRY_AIR, RAIN_N0, WATER_DENSITY, MASS_COEFF
and the helper functions air_density and write_var are duplicated between
generate_fake_wrf.jl and wrf_dualpol_radar.jl; extract them into a new shared
module (e.g., physics or shared) to avoid drift, move the constant definitions
and the implementations of air_density and write_var into that module, export
those symbols, and update both scripts to import the module and reference
R_DRY_AIR, RAIN_N0, WATER_DENSITY, MASS_COEFF, air_density, and write_var
instead of redefining them.
In `@examples/rain_radar.jl`:
- Around line 42-65: In water_refractive_index replace the hardcoded
speed-of-light literal (2.99792458e8) used in the εimag term with the existing
c0_m_per_s() helper for consistency with ice_refractive_index; specifically
update the σ * λ_m / (2π * 2.99792458e8 * ε0) expression inside
water_refractive_index to use c0_m_per_s() so the computation reads σ * λ_m /
(2π * c0_m_per_s() * ε0), keeping all other math unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4d98a2ec-d784-43de-988b-6e4a991137c2
📒 Files selected for processing (14)
.gitignoreCHANGELOG.mdProject.tomlREADME.mddocs/make.jldocs/src/examples/rain_radar.mddocs/src/index.mdexamples/README.mdexamples/radiative_transfer/Project.tomlexamples/radiative_transfer/README.mdexamples/radiative_transfer/generate_fake_wrf.jlexamples/radiative_transfer/wrf_dualpol_radar.jlexamples/rain_radar.jlpackages/EBCMPrecisionLossEstimators/Project.toml
be0241f to
0ac611e
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/radiative_transfer/generate_fake_wrf.jl`:
- Around line 48-53: In parse_args, after parsing nx, ny, nz, and nt (the
variables in the tuple returned by parse_args), add validation to ensure each is
> 0 and if any value is <= 0 raise a clear CLI error (e.g., throw or call
error/exit with a descriptive message mentioning which dimension is invalid and
its value); update parse_args to perform this check before returning so invalid
grid sizes are rejected early (reference the parse_args function and the
nx/ny/nz/nt variables).
In `@examples/radiative_transfer/wrf_dualpol_radar.jl`:
- Around line 63-68: The CLI currently allows --output to equal --input which
leads to destructive deletion of the source when the code later removes the
output path; update the argument handling around opts["input"]/opts["output"]
(variables input and output in the shown block) to detect input == output and
either error out with a clear message or handle safely by writing to a temporary
file/path and atomically replacing the input after successful write; ensure any
deletion logic that currently unconditionally removes output (the code around
the remove/delete call) is only executed after confirming output != input or
after a successful temp-to-target rename.
- Around line 267-272: The code currently writes Float32(NaN) when
radar_moments(...) returns invalid values; instead detect invalid moments (e.g.,
use isnan or !isfinite on moments.ZH / moments.ZV / moments.ZDR / moments.RHOHV
/ moments.KDP or a single validity check after calling radar_moments) and assign
the FILL sentinel (e.g., FILL) to ZH[idx], ZV[idx], ZDR[idx], RHOHV[idx],
KDP[idx] when invalid; otherwise continue converting valid values with
Float32(moments.<field>); update the block that calls radar_moments and sets
ZH/ZV/ZDR/RHOHV/KDP accordingly to implement this conditional assignment.
- Around line 256-257: The loop is materializing all CartesianIndex objects by
using collect(CartesianIndices(fields.qrain)), causing a large allocation;
change the loop to iterate directly over CartesianIndices(fields.qrain) (i.e.
Threads.@threads for idx in CartesianIndices(fields.qrain)) so idx remains a
CartesianIndex and you avoid allocating an intermediate array when indexing
fields.qrain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e320d4dc-97c8-40b1-a461-dc22938621d4
📒 Files selected for processing (14)
.gitignoreCHANGELOG.mdProject.tomlREADME.mddocs/src/examples/rain_radar.mddocs/src/index.mdexamples/README.mdexamples/radiative_transfer/Project.tomlexamples/radiative_transfer/README.mdexamples/radiative_transfer/generate_fake_wrf.jlexamples/radiative_transfer/shared.jlexamples/radiative_transfer/wrf_dualpol_radar.jlexamples/rain_radar.jlpackages/EBCMPrecisionLossEstimators/Project.toml
✅ Files skipped from review due to trivial changes (7)
- docs/src/index.md
- examples/radiative_transfer/README.md
- examples/radiative_transfer/Project.toml
- README.md
- .gitignore
- examples/README.md
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/EBCMPrecisionLossEstimators/Project.toml
- Project.toml
- examples/rain_radar.jl
0ac611e to
39c2139
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/radiative_transfer/wrf_dualpol_radar.jl`:
- Around line 80-88: parse_range currently returns an empty Float64 vector for
descending or degenerate specs which lets invalid CLI input silently pass;
update parse_range to validate the produced bins and fail fast: after building
the candidate range in parse_range (both the two-part and three-part branches)
check for invalid step (e.g., zero) and then collect the range into a vector and
if isempty(vector) raise an error like "range must produce at least one bin";
ensure you reference the parse_range function and its two branches
(length(parts)==2 and length(parts)==3) so the fix covers both forms.
- Around line 155-168: The cache logic in load_or_compute_scattering_table
should handle corrupt/unreadable files and missing table entries: wrap
deserialize(cache) in a try/catch and on any error or if
scattering_cache_matches(payload, Ds, freq_hz, solver_params) returns true but
payload does not contain a valid :table, log a warning and continue to compute
table = scattering_table(Ds, freq_hz; solver) (and still call serialize when
cache is provided). Also add validation in parse_range to ensure Ds is non-empty
(and that descending/degenerate inputs produce an explicit error or early exit
with a clear message) so the CLI does not proceed writing sentinel-only outputs;
surface that error to the caller so higher-level code aborts instead of
producing FILL-only results.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 543db409-2148-477d-b895-79b60992f65e
📒 Files selected for processing (14)
.gitignoreCHANGELOG.mdProject.tomlREADME.mddocs/src/examples/rain_radar.mddocs/src/index.mdexamples/README.mdexamples/radiative_transfer/Project.tomlexamples/radiative_transfer/README.mdexamples/radiative_transfer/generate_fake_wrf.jlexamples/radiative_transfer/shared.jlexamples/radiative_transfer/wrf_dualpol_radar.jlexamples/rain_radar.jlpackages/EBCMPrecisionLossEstimators/Project.toml
✅ Files skipped from review due to trivial changes (8)
- README.md
- docs/src/index.md
- packages/EBCMPrecisionLossEstimators/Project.toml
- Project.toml
- .gitignore
- examples/radiative_transfer/README.md
- CHANGELOG.md
- examples/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- examples/radiative_transfer/Project.toml
- examples/radiative_transfer/shared.jl
- examples/radiative_transfer/generate_fake_wrf.jl
- examples/rain_radar.jl
Add an optional WRF-like dual-pol radar workflow with synthetic NetCDF data generation, and bump the package plus dependent compat bounds to 0.5.
39c2139 to
95d408e
Compare
Summary by CodeRabbit
New Features
Documentation
Chores
Style