Impedance tube: complete circular, rectangular and square cross-section support - #380
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThe impedance-tube API now supports square and rectangular geometry, ASTM-specific frequency checks, hydraulic diameter calculation, retained solver context, context-aware transfer-matrix plotting, and tube-shape metadata in ISO reports. Public exports, documentation, translations, and tests are updated. ChangesImpedance-tube API and reporting
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Solver
participant RangeCheck
participant TransferMatrix
participant Plot
Solver->>RangeCheck: validate frequency and tube geometry
RangeCheck-->>Solver: issue warning when outside ASTM range
Solver->>TransferMatrix: retain measurement context
Plot->>TransferMatrix: request stored plotting values
TransferMatrix-->>Plot: return frequency and characteristic impedance
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #380 +/- ##
=======================================
Coverage 96.82% 96.82%
=======================================
Files 218 218
Lines 30144 30210 +66
=======================================
+ Hits 29188 29252 +64
- Misses 956 958 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Numerical conformance report✅ 427/427 conformance checks pass across 53 domains and 278 standards - filters class 1 - weightings within IEC 61672-1 class 1. Each row pins a standard clause to its expected normative value and the value the library computes. Every section below is collapsible and stays collapsed while all of its rows pass; a section with any failing row opens automatically. ✅ Numerical validation - filters & weightings: class showcase (IEC 61260-1 · IEC 61672-1 · ISO 7196)IEC 61260-1:2014 class per filter architecture (order 6, one-third-octave, 100 Hz-10 kHz, fs = 48 kHz). For each architecture the table shows, at its binding band, the measured relative attenuation and the class-1 limit it must clear, so the number and the range it must sit in are both visible. A positive margin means the acceptance limits are met with that much room.
Only Butterworth (the library default) and Chebyshev-II are class-compliant architectures. Chebyshev-I and elliptic trade the mask for passband ripple, and Bessel for a maximally-flat group delay (soft rolloff); they cannot satisfy the IEC 61260-1 Class 1/2 attenuation mask by construction, so they are labelled By design - this is expected, not a failure or regression. Frequency-weighting conformance (A/C: IEC 61672-1 Table 3; G: ISO 7196 A.3). The max deviation from nominal is informational (it falls at a frequency extreme where the tolerance is widest and asymmetric); compliance is judged at the binding frequency - the one with the least headroom - where the deviation, the applicable tolerance band and the headroom are shown together.
✅ Filters & weightings: 100% (10/10)
✅ Levels & dosimetry: 100% (6/6)
✅ Room acoustics: 100% (12/12)
✅ Psychoacoustics: 100% (12/12)
✅ Speech transmission (IEC 60268-16): 100% (10/10)
✅ System measurement (Golay / Kirkeby / Mueller-Massarani): 100% (5/5)
✅ Intensity & sound power: 100% (6/6)
✅ Room & building acoustics: 100% (52/52)
✅ Building prediction & uncertainty: 100% (15/15)
✅ Outdoor propagation & occupational exposure: 100% (10/10)
✅ Materials: absorption, airflow & impedance: 100% (6/6)
✅ Scattering & diffusion (ISO 17497): 100% (14/14)
✅ In-situ road absorption (ISO 13472): 100% (3/3)
✅ Precision sound power (ISO 3745 / 9614-3): 100% (4/4)
✅ Human vibration (ISO 8041 / 2631 / 5349): 100% (15/15)
✅ Speech intelligibility (ANSI S3.5-1997): 100% (7/7)
✅ Objective intelligibility (STOI / ESTOI): 100% (3/3)
✅ Impulsive-sound prominence (NT ACOU 112): 100% (2/2)
✅ Impulsive-sound prominence (ISO/PAS 1996-3): 100% (2/2)
✅ Room noise (ANSI S12.2-2019): 100% (3/3)
✅ Hearing threshold (ISO 7029 / ISO 389-7): 100% (3/3)
✅ Measurement uncertainty (GUM / Supplement 1): 100% (7/7)
✅ Noise-induced hearing loss (ISO 1999): 100% (6/6)
✅ Multiple-shock whole-body vibration (ISO 2631-5): 100% (6/6)
✅ Sound absorption in enclosed spaces (EN 12354-6): 100% (2/2)
✅ Prominent discrete tones (ECMA-418-1): 100% (2/2)
✅ Tonal audibility (ISO/PAS 20065): 100% (11/11)
✅ Psychoacoustic annoyance & fluctuation strength (Fastl & Zwicker): 100% (3/3)
✅ Electroacoustics: distortion & frequency response: 100% (20/20)
✅ Calibrated spectral analysis (Bendat & Piersol): 100% (12/12)
✅ Multiple-input coherence (Bendat & Piersol): 100% (5/5)
✅ Time-frequency analysis (Bendat & Piersol): 100% (3/3)
✅ Correlation, time delay and envelope (B&P / Knapp & Carter): 100% (7/7)
✅ Cepstrum, liftering and envelope spectrum (Havelock / B&P): 100% (3/3)
✅ Time synchronous averaging (McFadden 1987): 100% (5/5)
✅ Data qualification and Rice statistics (Bendat & Piersol): 100% (8/8)
✅ Underwater acoustics (ISO 18405/17208/18406): 100% (6/6)
✅ Underwater sound propagation (transmission loss): 100% (15/15)
✅ Underwater numerical propagation (modes / rays / PE): 100% (4/4)
✅ Aircraft noise (ICAO Annex 16 / IEC 61265): 100% (15/15)
✅ Rotorcraft noise (ECAC Doc 32 / NORAH2): 100% (12/12)
✅ Wind-turbine noise (IEC 61400-11): 100% (3/3)
✅ Porous & multilayer absorbers (Mechel / Bies / Cox & D'Antonio): 100% (10/10)
✅ Slow-sound perfect absorbers (Jimenez et al. Appl. Sci. 2017): 100% (3/3)
✅ Program loudness (ITU-R BS.1770 / EBU R 128): 100% (8/8)
✅ 2D FDTD wave simulation (Attenborough & Van Renterghem 2021, Ch. 4): 100% (2/2)
✅ Swept-sine distortion & phase utilities (Farina / Novak): 100% (7/7)
✅ Spherical ground & barriers (Attenborough / Salomons / Bies): 100% (7/7)
✅ Panel & aperture sound insulation (Bies / Hopkins / Cremer): 100% (11/11)
✅ Bending-wave plate-junction transmission (Cremer / Craik / Hopkins): 100% (6/6)
✅ Atmospheric refraction (Salomons rays / GFPE): 100% (3/3)
✅ Electroacoustics: 100% (6/6)
✅ Industrial noise control: 100% (9/9)
Tests & coverage — 33612 tests, 0 failures (✅ all green)
Conformance harness: |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@site/src/content/docs/reference/api/materials/impedance-tube.md`:
- Around line 872-875: Update the documentation wording around the matrix
frequency and air characteristic impedance defaults to clarify that only
hand-built matrices lack this context and require explicit values. State that
solver-generated matrices retain self.frequency and
self.air_characteristic_impedance, so .plot() does not always require those
arguments.
In `@src/phonometry/_report/metadata.py`:
- Around line 144-147: Preserve the positional constructor order of the
ReportMetadata dataclass by moving the new tube_shape field after the existing
notes field, leaving thickness and all prior fields unchanged. Then regenerate
the API reference documentation.
In `@src/phonometry/materials/impedance_tube.py`:
- Around line 837-874: Update _warn_astm_plane_wave to validate that both s1 and
s2 are positive before calculating k_upper or k_lower, raising the established
clear ValueError for invalid spacing. Keep the existing diameter validation and
warning behavior unchanged so direct callers such as transfer_matrix_two_load
and transfer_matrix_one_load receive the same validation.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ada2f912-8d80-4ed2-a064-ba12f5ac393e
📒 Files selected for processing (13)
docs/api-reference.mddocs/materials.mdsite/src/content/docs/es/guides/materials.mdxsite/src/content/docs/guides/materials.mdxsite/src/content/docs/reference/api/building/insulation.mdsite/src/content/docs/reference/api/materials/impedance-tube.mdsrc/phonometry/__init__.pysrc/phonometry/_report/_i18n.pysrc/phonometry/_report/iso10534.pysrc/phonometry/_report/metadata.pysrc/phonometry/materials/__init__.pysrc/phonometry/materials/impedance_tube.pytests/materials/test_impedance_tube.py
…on support The ISO 10534-2 branch already distinguished circular from rectangular tubes; the ASTM E2611 branch ignored the cross-section entirely. Both branches now cover it end to end: - Accept "square" as a cross-section alias (the rectangular bound with d the side length) in plane_wave_frequency_range() and two_microphone_impedance(). - New plane_wave_frequency_range_astm(): ASTM E2611-19 working range with the unrounded circular constant (fu d < 0.586 c, 6.2.4.1), the rectangular bound (0.500 c, 6.2.5), the spacing bound (fu s < 0.40 c, 6.5.4) and the 1 % of wavelength lower limit (6.2.3). - wave_decomposition(), transfer_matrix_two_load() and transfer_matrix_one_load() accept diameter/shape and emit an ImpedanceTubeWarning outside the plane-wave range (wavenumber-space check, no speed of sound needed). - New hydraulic_diameter(width, height) helper (4A/P) for the tube_attenuation_constant() rectangular case (ISO 10534-2 A.2.1.5). - ImpedanceTubeResult retains spacing/x1/diameter/shape and TransferMatrix retains l1/s1/l2/s2/thickness/diameter/shape plus the frequency vector and air characteristic impedance, so TransferMatrix.plot() now works without arguments on solver-built matrices. - ReportMetadata.tube_shape prints a translated cross-section row on the ISO 10534-2 fiche. - Tests for every new path; materials guide (EN/ES), api-reference and generated API pages updated.
…onstant - Append ReportMetadata.tube_shape after the original fields so the public positional constructor order is unchanged. - Validate s1/s2 inside _warn_astm_plane_wave so a zero spacing raises ValueError instead of ZeroDivisionError from the two transfer-matrix solvers. - Reword TransferMatrix.plot() docstring: solver-built matrices retain frequency and rho c, only hand-built ones must pass them. - Hoist the duplicated diameter validation message into a module constant (SonarCloud S1192).
jmrplens
force-pushed
the
tube-cross-section
branch
from
July 27, 2026 04:01
596edc5 to
24552b9
Compare
github-actions
Bot
added
documentation
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The ISO 10534-2 branch already distinguished circular from rectangular cross-sections in
plane_wave_frequency_range(); the ASTM E2611 four-microphone branch ignored the tube section entirely, and the rectangular path had no tests. This PR completes cross-section support in both branches.Changes
shapenow acceptscircular | rectangular | square(a square tube is the rectangular bound with d the side length, per ISO 10534-2 4.1 and ASTM E2611-19 6.2.5) inplane_wave_frequency_range()andtwo_microphone_impedance().plane_wave_frequency_range_astm(): the ASTM E2611-19 working range, which differs from the ISO one on every constant: the unrounded circular cut-on fu d < 0.586 c (6.2.4.1), the rectangular 0.500 c with d the largest section dimension (6.2.5), the spacing bound fu s < 0.40 c (s no larger than 80 % of c/2fu, 6.5.4) and a lower limit at 1 % of the wavelength (6.2.3).wave_decomposition(),transfer_matrix_two_load()andtransfer_matrix_one_load()accept optionaldiameter/shapeand emit anImpedanceTubeWarningoutside the plane-wave range. The check runs on the real part of the wavenumber (k s < 0.80 pi, k d < 2 pi K, k s > 0.02 pi), so no speed of sound is needed; the upper spacing bound binds the largest microphone spacing and the lower one the smallest.hydraulic_diameter(width, height): the 4A/P diameter thattube_attenuation_constant()expects for rectangular tubes (ISO 10534-2 A.2.1.5); previously the docstring told the user to compute it by hand.ImpedanceTubeResultkeepsspacing/x1/diameter/shapeandTransferMatrixkeepsl1/s1/l2/s2/thickness/diameter/shapeplus the frequency vector and the air characteristic impedance.TransferMatrix.plot()therefore works with no arguments on solver-built matrices; hand-built matrices (e.g.air_layer_transfer_matrix) still pass them explicitly. All new fields are trailing keyword defaults, and every constructor call in the repo is keyword-only, so this is backward compatible.ReportMetadata.tube_shapefield (validated) prints a translated cross-section row on the ISO 10534-2 report.Verification
plot()path.Summary by CodeRabbit
tube_shapeto impedance-tube report metadata.