pyroomacoustics
pyroomacoustics copied to clipboard
Raise warning when extruding Shoebox
Dear developers, I just noticed that, when extruding a 2D shoebox, the default absorptions for the floor and the ceiling are set to 0. Since it is very useful to design room is 2D and then extrude them, it would be be useful to have one of following behaviors:
- set the new walls' abs. profile to the one used for the other walls and raise a warning
- set the new walls' abs. profile to 0 and raise a warning.
With Shoebox class
W, L, H = 5, 4.5, 3.1
room = pra.ShoeBox([W, L], fs=fs, absorption=0.99, max_order=1, sigma2_awgn=0.000000)
print([room.walls[w].absorption for w in range(len(room.walls))])
room.extrude(H)
print([room.walls[w].absorption for w in range(len(room.walls))])
output:
[0.9998999834060669, 0.9998999834060669, 0.9998999834060669, 0.9998999834060669]
[0.9998999834060669, 0.9998999834060669, 0.9998999834060669, 0.9998999834060669, 0.0, 0.0]
With Room class
corners = np.array([[0,0], [0,3], [5,3], [5,1], [3,1], [3,0]]).T # [x,y]
room = pra.Room.from_corners(corners, absorption=0.99)
print([room.walls[w].absorption for w in range(len(room.walls))])
room.extrude(3.)
print([room.walls[w].absorption for w in range(len(room.walls))])
output:
[0.9998999834060669, 0.9998999834060669, 0.9998999834060669, 0.9998999834060669]
[0.9998999834060669, 0.9998999834060669, 0.9998999834060669, 0.9998999834060669, 0.0, 0.0]
Ah, I noticed now the warning about absorption being deprecated. Maybe I should always materials
Thanks for raising this point.
I'm ready between the lines that you spent some time debugging a bug due to this behavior 🙇
The problem is that no default value is always what you want.
What do you think about alternative strategies
- make the materials argument mandatory for the
extrude
method - set to average absorption to 1.0 instead so that the new room is consistent with the 2D one
Also, please use the materials
argument instead. absorption
has been deprecated for a long time. Beware that the definition of absorption
is different from that of energy_absorption
of a material.
Dear @fakufaku,
I'm ready between the lines that you spent some time debugging a bug due to this behavior 🙇
Ahah, no, no worries :sweat_smile: For fast prototyping, I usually copy and paste code snippets from the examples and the notebooks. They are such good resources, maybe they should be updated.
What do you think about alternative strategies 1. make the materials argument mandatory for the extrude method
I think solution 1. Why not solution 2.? Because at this point, it is better to avoid any further use of the absorption
argument.
Making material
mandatory in the extrude
method may add some complexity, but it will rise awareness and avoid bugs.
Cheers :bow:
Thank you for this amazing library