search

DavidDiazGuerra / gpuRIR

Python library for Room Impulse Response (RIR) simulation with GPU acceleration
GNU Affero General Public License v3.0
488 stars 94 forks source link

Bug in 'assert mic_pattern is "omni" or orV_rcv is not None' #19

Closed quancs closed 3 years ago

quancs commented 3 years ago

when mic_pattern is loaded from a json file, and equals to 'omni', simulateRIR raises an exception: Traceback (most recent call last): File "<string>", line 1, in <module> File "D:\ProgramData\Miniconda3\envs\torch\lib\site-packages\gpurir-1.2.0-py3.8-win-amd64.egg\gpuRIR\__init__.py", line 146, in simulateRIR assert mic_pattern is "omni" or orV_rcv is not None, "the mics are not omni but their orientation is undefined" AssertionError: the mics are not omni but their orientation is undefined

Maybe we can modify the code below assert mic_pattern is "omni" or orV_rcv is not None to assert mic_pattern == "omni" or orV_rcv is not None to avoid the bug.

quancs commented 3 years ago

when I executes mic_pattern is "omni", the terminal returns

<string>:1: SyntaxWarning: "is" with a literal. Did you mean "=="?
False

where mic_pattern is loaded from a json file

DavidDiazGuerra commented 3 years ago

Hello,

I think you're right, using is in that string comparison wasn't a great idea. Have you tried if the solution you proposed (using == instead of is) works? I can change that if it does. If you prefer it, I could create a new branch with that change so you can install it from GitHub and see if it works.

Best regards, David

quancs commented 3 years ago

Hello,

I think you're right, using is in that string comparison wasn't a great idea. Have you tried if the solution you proposed (using == instead of is) works? I can change that if it does. If you prefer it, I could create a new branch with that change so you can install it from GitHub and see if it works.

Best regards, David

Yes, I tested it in terminal. The loaded mic_pattern == "omni", but not is "omni"

DavidDiazGuerra commented 3 years ago

I think it would be better to test it within the whole library before pushing it to the master branch. I've created a new branch with this change, I think you should be able to install it by:

pip install https://github.com/DavidDiazGuerra/gpuRIR/zipball/fix-issue-19

If you confirm that it works fine, I'll include it in the master branch.

quancs commented 3 years ago

@DavidDiazGuerra It works. I confirmed that it is fixed.

DavidDiazGuerra commented 3 years ago

Thanks for checking it out. I'll pull the fix to the fix to the main branch.