Skip to content

Conversation

@ilsuono
Copy link

@ilsuono ilsuono commented Nov 5, 2025

Which issue(s) are closed by this pull request?

Closes #41

Changes proposed in this pull request:

  • Add interval_specific_decay_time() in pyrato.parameters
  • Add corresponding unit tests

@ilsuono
Copy link
Author

ilsuono commented Nov 5, 2025

TODO: tests; Please review deeply otherwise 🙏🏻

Copy link
Member

@ahms5 ahms5 left a comment

Choose a reason for hiding this comment

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

Thanks a lot. Looks very professional. For some of my comment I'm not very confident. Please comment you have sth else in mind.

pyrato/edc.py Outdated
energy_decay_curve.comment)


def _edc_linregress(energy_decay_curve: pf.TimeData, t0: float, t1: float):
Copy link
Member

Choose a reason for hiding this comment

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

It might be interesting for users to also use this method directly. I would suggest to make it public

pyrato/edc.py Outdated
energy_decay_curve.times[t0_idx:t1_idx],
energy_decay_curve.time[t0_idx:t1_idx])

return slope, intercept
Copy link
Member

Choose a reason for hiding this comment

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

Im not to familiar with the details, but it might make sence to already return the reverberation_time relative to 60db decay here.

pyrato/edc.py Outdated
return slope, intercept


def early_mid_decay_time(energy_decay_curve: pf.TimeData):
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this limits are defined in a Standard or sth similar. Maybe @mberz can claryfiy.
The issue might just provide an example on how this general method could be used. So that the aim could just be to provide the upper method.

@ilsuono ilsuono marked this pull request as draft November 6, 2025 10:00
@ilsuono ilsuono marked this pull request as ready for review November 6, 2025 10:45
@ilsuono ilsuono requested a review from ahms5 November 6, 2025 10:45
@ahms5 ahms5 requested review from a team, SimonBuechner, artur-pa, f-brinkmann, hoyer-a and mberz and removed request for a team, SimonBuechner, artur-pa, f-brinkmann and hoyer-a November 11, 2025 10:12
@ahms5 ahms5 added the ASSA 2025 Issues and PRs related to the Open Source Software school at ASSA 2025 label Nov 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ASSA 2025 Issues and PRs related to the Open Source Software school at ASSA 2025

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants