Repository navigation
Conversation
The name given to generated objects was read by a block copied in 20 places that walked the stack with traceback.walk_stack(None). Since Python 3.12 it starts four frames up instead of two, so in a plain script it walks past the end of the stack and pytta.generate.sweep() raises AttributeError at module level and UnboundLocalError inside a function. One helper, _instanceinfo.caller_name, replaces the copies: it walks from the caller explicitly and returns an empty name when the source line is not available (python -c). Tests run real scripts in a subprocess, since under pytest the stack is deep enough to hide the failure. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
scipy.signal.hann and scipy.signal.tukey were removed in SciPy 1.13. The five calls now use scipy.signal.windows, the same functions with the same defaults. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…upport RoomAnalysis took C80, D50, STearly and STlate from the energy decay curve of each band filtered response, so the delay of the band filters moved early energy past the time limits (about 1 dB low C80 at 125 Hz). The broadband response is now windowed before the filtering, as ISO 3382-1:2009, A.3.4, recommends, with the Lundeby truncation and tail correction kept for the late windows. STlate now ends at 1000 ms (ISO 3382-1, C.2), and bypassLundeby=True no longer raises UnboundLocalError. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This branch has not been deployed
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
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
RoomAnalysisunderestimates C80 and D50 in the low frequency bands, by about 1 dB at 125 Hz in octave bands. The parameters are computed on the band filtered response with the time limits counted from the start of the broadband response, so the delay of the band filters moves early energy past the 80 ms or 50 ms limit. ISO 3382-1:2009, A.3.4, describes this problem and recommends windowing the broadband impulse response before the filtering. This pull request computes C80, D50, STearly and STlate that way and keeps the Lundeby noise compensation for the late part.The problem
RoomAnalysiscrops the broadband response at its start (the first sample within 20 dB of the maximum, as ISO 3382-1 asks), filters it into bands with the causal Butterworth filters ofOctFilter, and takes C80, D50, STearly and STlate from the energy decay curve of each filtered band. The filters delay the energy, and more so in the narrow low frequency bands. Defined as in A.3.4, the time for half the energy of the filter impulse response, the delay of the order 4 octave filters at 48 kHz is 11.0 ms at 125 Hz, 5.5 ms at 250 Hz, 2.8 ms at 500 Hz and 1.4 ms at 1 kHz. At 125 Hz an 80 ms limit therefore falls on energy that arrived at about 69 ms.To measure the effect, 20 synthetic impulse responses were analysed: exponentially decaying white noise with T = 1 s, whose expected C80 is 10 lg(e^(13.8155 × 0.08) − 1) = 3.05 dB in every band. The reference is the method of A.3.4: the first 80 ms and the rest of the broadband response are filtered separately with the same filters, and all the energy of each output is summed. The mean difference from that reference, in dB, with its standard error:
RoomAnalysisbefore this changeita_roomacousticsThe ITA-Toolbox stays close to the reference because it filters the bands with zero-phase filters by default. T30 is not affected, and both tools agree on it.
The change
windowed_band_energy(ir, start, stop, compensation, **kwargs)inpytta/classes/analysis.pyreturns the energy in each band of the broadband response betweenstartandstop, windowed before the filtering. One second of zeros is appended before the filtering, so the energy that the filters delay past the end of the response is kept.RoomAnalysis.estimate_energy_parameterscomputes C80 and D50 from the windowed early and late energies, as A.3.4 recommends. STearly (20 to 100 ms over 0 to 10 ms) and STlate (100 to 1000 ms over 0 to 10 ms) are computed in the same way.cumulative_integrationnow records, for each band, the Lundeby truncation index and the tail correction C, and the filtered late window is cut at that index and C is added, exactly as the energy decay curve does. A new optional argument,compensation, carries these values, and the return value ofcumulative_integrationis unchanged.Other fixes
RoomAnalysis(..., bypassLundeby=True)raisedUnboundLocalError, becauseenergy_decay_calculationreturnedlundebyParamswithout defining it in that case. It is nowNonethere.Unchanged
definition,clarity,st_earlyandst_latestay in the module for other callers, althoughRoomAnalysisno longer uses them._start_sample_ISO3382returns sample 1 when no sample before the maximum lies 20 dB below it, so a response that starts at its first sample loses that sample. [FIX] start an impulse response at sample 0 when nothing precedes its peak #128 fixes it; the test with impulses here places 10 ms of silence before the first one, so it holds with and without [FIX] start an impulse response at sample 0 when nothing precedes its peak #128.Interaction with #58
#58 adds the noise compensation methods of ISO 3382-1 to
energy_decay_calculation(edcMethod, including'subtractNoise'), and both pull requests change that function.windowed_band_energyalready accepts a third element in the compensation of each band, the noise energy per sample, which it subtracts over the late window. Once both are merged,energy_decay_calculationreturns(interIdx, C, BGL)for'subtractNoise'and(interIdx, C, 0)otherwise, andRoomAnalysispassesedcMethodandcompensationtogether. My integration branch holds that combination, where the tests of both pull requests pass.Tests
The tests in
tests/test_room_energy_windowing.pyprint the measured values:OctFilterandscipy.signal.sosfilt, within 0.005 dB for three responses. C80 is rounded to 0.01 dB.This pull request builds on #52, without which
RoomAnalysiscannot run on Python 3.12, and shows its commit until #52 is merged. On top of #52 the 3 failures intest_pyttasave.pythat #60 fixes are the same with and without this change. Checked with Python 3.12, numpy 1.26 and scipy 1.17, and against the ITA-Toolbox in MATLAB R2026b.Co-Authored-By: Claude Fable 5.1 noreply@anthropic.com