Add room generator.

[ebezzam]

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".

Happy PR 😃

Ran nosetests and I got the following error:

ERROR: Failure: ImportError (bad magic number in 'pyroomacoustics.c_package': b'\x03\xf3\r\n')

[fakufaku]

Very exciting! Thanks for the contribution. I think we should really think how to get this right, especially in terms of API, how it fits and should be used as part of the package, and keep it extensible.

Currently, your implementation is part of datasets and essentially creates a dataset of room impulse response.

Separate random room generation from dataset creation if possible

I was thinking of having a random sub-package could be nice, and we could call pra.random.room() or pra.random.shoebox() to get a random room. These could get extra arguments like pra.random.room(n_sources=2, mic_array=my_mic_array). These could be supported by classes implementing generators underneath. By having a generator, we could implement cool things like

room_gen = pra.random.ShoeBoxGenerator()
for room in room_gen:
    room.simulate()
    res = algorithm(room.mic_array.signals)
    save(res)

Of course, there are many questions, like how do we define the signals of the sources for example. Do we provide a dataset as argument to the RoomGenerator object ? Do we provide the shape of the microphone array to the RoomGenerator object ?

Of course we would like to support some form of seeding to have repeatable room sequences, for example via numpy random number generator (get_state, set_state).

Once this is implemented, I think it is quite easy to add the dataset part which would essentially just be a wrapper around the room generator described above. In fact, I think the current implementation is close to that in class structure.

I am not suggesting that all of this be implemented as part of this PR. But we need to be careful that we get the API right. Things to look out for in particular:

• Do not use an API name that we'd like to use for something more general (or different) later on
• Do not set too specific defaults that might force us to change the API later (e.g. the default fs in room)

One thing we could do is to add the all the API that we would eventually like to have, and raise NotImplemented exceptions when called. Alternatively, these could be abstract classes.

class RandomRoomGenerator(object):
    def __init__(self, source_generator=None, noise_generator=None):
        raise NotImplementedError

Other details

In the example script, you use the click package, which seems great, but requires us to add an extra requirement (for the examples). If the interface can be equally written with argparse, I would suggest to switch to that instead.

Travis build failing

This seems to be an issue with the ABCMeta class in python 2.7 only

Traceback (most recent call last):
  File "/Users/travis/miniconda/envs/test_env/lib/python2.7/site-packages/nose/loader.py", line 418, in loadTestsFromName
    addr.filename, addr.module)
  File "/Users/travis/miniconda/envs/test_env/lib/python2.7/site-packages/nose/importer.py", line 47, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/Users/travis/miniconda/envs/test_env/lib/python2.7/site-packages/nose/importer.py", line 94, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/Users/travis/build/LCAV/pyroomacoustics/pyroomacoustics/__init__.py", line 137, in <module>
    from . import datasets
  File "/Users/travis/build/LCAV/pyroomacoustics/pyroomacoustics/datasets/__init__.py", line 144, in <module>
    from .room import ShoeBoxRoomGenerator
  File "/Users/travis/build/LCAV/pyroomacoustics/pyroomacoustics/datasets/room.py", line 32, in <module>
    from pyroomacoustics.datasets.distribution import DiscreteDistribution, \
  File "/Users/travis/build/LCAV/pyroomacoustics/pyroomacoustics/datasets/distribution.py", line 29
    class Distribution(metaclass=ABCMeta):
                                ^
SyntaxError: invalid syntax

Appveyor build fail

This one seems to be windows/conda related. This issue seems related, especially the following

Update: it looks like the problem is resolved if I run activate base before running conda build. Evidently whatever was working for me before was fragile and I wasn't supposed to rely on it. Note that I have C:\Anaconda3 and C:\Anaconda3\Scripts in my user PATH.

So maybe you can try adding conda activate base after line 42 in .appveyor.yml.

[ebezzam]

I like the idea of having this separate / more general RoomGenerator object, which could be an abstract class that takes as input a source generator and noise generator, with this example here being a subclass for ShoeBox rooms / the approach from this paper. (And another example/class can be made based off this paper).

I'll start off with creating the random sub-package and switching to argparse, and I'll also think more about how an API can be created with less assumptions.

[fakufaku]

I just saw your comment about bad magic number after running nosetests. This happen when the .pyc file is left over from a file that do not exist after switching branch. Here c_package relates to the C extension in master. You can manually delete the pyc files to get rid of the problem.

[ebezzam]

I just saw your comment about bad magic number after running nosetests. This happen when the .pyc file is left over from a file that do not exist after switching branch. Here c_package relates to the C extension in master. You can manually delete the pyc files to get rid of the problem.

Yes I found this solution online and deleting the pyc files was indeed the fix.