You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Thanks for sending a pull request (PR), we really appreciate that! Before hitting the submit button, we'd be really glad if you could make sure all the following points have been cleared.
Please also refer to the doc on contributing for more details. Even if you can't check all the boxes below, do not hesitate to go ahead with the PR and ask for help there.
Are there docstrings ? Do they follow the numpydoc style ?
Have you run the tests by doing nosetests or py.test at the root of the repo ?
Have you checked that the doc builds properly and that any new file has been added to the repo ? How to do that is covered in the documentation.
Is there a unit test for the proposed code modification ? If the PR addresses an issue, the test should make sure the issue is fixed.
Last but not least, did you document the proposed change in the CHANGELOG file ? It should go under "Unreleased".
Hi @dzejnwashere , thanks for the contribution. I think there is a possiblity that the room impulse responses (RIR) are actually not recalculated between the steps.
This is not very well documented, but pyroomacoustics stores the room impulse responses, and does not recompute them after the simulation is done. So updating the source locations is not sufficient to guarantee the RIR are updated.
I think this is why the signal scale was also not changed.
I apologize for the poor documentation on that part... 🙇
What I would suggest is to precompute all the source locations in advance, and then simulate all the RIRs at once. This would solve the issue and probably also make the code simpler.
Another thing is that room.py is already a monster at the moment and if possible I would like to avoid adding the moving sources in there.
Also, I don't think it is necessary for the code to be a method of room to work.
At first, would you be ok to change this code to be an example and add in the examples folder?
Finally, the code should be linted with black and isort.
Please consult CONTRIBUTING for instructions.
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
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.
Thanks for sending a pull request (PR), we really appreciate that! Before hitting the submit button, we'd be really glad if you could make sure all the following points have been cleared.
Please also refer to the doc on contributing for more details. Even if you can't check all the boxes below, do not hesitate to go ahead with the PR and ask for help there.
nosetestsorpy.testat the root of the repo ?Happy PR 😃