Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent ValueError: The source must be added inside the room raised #248

Open
thejasvibr opened this issue Jan 24, 2022 · 10 comments
Open

Comments

@thejasvibr
Copy link

Hi,
I'm trying to simulate ultra-sound propagation in a cave using a mesh of the LiDAR scan. The ...source must be added inside the room. error appears inconsistently:

"xx\source_not_in_room.py", line 67, in <module>
    room.add_source(bat1_call_emission_points[i])

  File "xx\anaconda3\envs\nussl\lib\site-packages\pyroomacoustics\room.py", line 1908, in add_source
    return self.add(SoundSource(position, signal=signal, delay=delay))

  File "xx\anaconda3\envs\nussl\lib\site-packages\pyroomacoustics\room.py", line 1761, in add
    raise ValueError("The source must be added inside the room.")

ValueError: The source must be added inside the room.

The points are far from the walls (unlike Issue #181) - based on visualising the trajectory in the mesh (see ball_in_cave.mp4 for video of the trajectories).

For my use case I'm adding multiple sources to the room in a loop, and it sometimes works, and sometimes doesn't. The STL file and code used to generate this error are in this repo.

System and package specs:
Windows 10, Python 3.8.12, pyroomacoustics 0.6.0

@thejasvibr
Copy link
Author

Follow up: I also ran the same kind of STL based simulation inside a sphere - and the code ran without problems. Will close this issue as the cause is unclear.

Without knowing further, I'm now guessing it may have to do with the cave scan STL file. Sorry for the bother!

@fakufaku
Copy link
Collaborator

Thanks for reporting! Have you made sure the normals of the walls are all pointing outward from the room ? I think this is important for the check to work properly.

@thejasvibr
Copy link
Author

thejasvibr commented Jan 25, 2022

Edit: Incorrect/Wrong observation Can confirm it is because of the mesh! The mesh had a rather complex structure and some odd overhanging triangles etc. - after doing a lot of cleaning on the mesh - the sound propagation now runs smoothly.

@thejasvibr
Copy link
Author

Can confirm it is because of the mesh! The mesh had a rather complex structure and some odd overhanging triangles etc. - after doing a lot of cleaning on the mesh - the sound propagation now runs smoothly.

Actually not...the inconsistency remains...I have ensured that the face normals are all facing outside, and yet still get the reported issue. Yesterday was a somewhat lucky day I guess.

I took a look at the source code in room.py which has the class function is_inside - which checks if the source is in the mesh by counting the number of intersecting walls between a test point and the source point. By adding some print statements along lines 2401-2408 I extracted the randomly generated test points. I then visualised the lines drawn between test point and source using PyVista. The lines seemed to intersect the mesh/walls as expected (single wall intersection).

Furthermore, the code seems to reach the last else (line 2407) clause in lines 2401-2408 of room.py before returning False, which in turn raises the ValueError reported (snippet copied with my own comments below).

 # start over when ambiguous 
            if ambiguous:
                it += 1
                continue

            else:
                if is_on_border and not include_borders: # line 2401
                    return False
                elif is_on_border and include_borders:
                    return True
                elif count % 2 == 1:
                    return True
                else:   # line 2407 --> this else clause
                    return False

Could not dig deeper into the Cpp portions of the code as I have no experience in the language. The puzzling part is that in my use-case the microphones are placed at the same coordinates as the sources. The add_microphone_array method passes without raising errors, while the add_source method raises the error.

The repo is now updated to hold both the .blend, .ply and .stl files of the cleaned mesh from Blender. Would appreciate any help on this matter.

@thejasvibr thejasvibr reopened this Jan 26, 2022
@thejasvibr
Copy link
Author

Made some progress with troubleshooting the potential causes.
Even though the mesh had no holes (according to my manual inspections), and all visible faces were pointing 'outside' - the whole mesh as an object was non manifold. Perhaps the non-manifold (e.g. hidden faces, bowties, overhanging faces) nature of the mesh caused issues?

After running through the mesh with pymeshfix, the new manifold mesh now runs without raising errors. While the simulations work now (yay!), there're still some open questions about what exactly caused the previous errors to be raised. The random line method to check if the line runs through an odd number of walls/faces raised errors even when it passed through okay looking faces (a 'normal' single face) of the mesh.

Does room.py also depend on the mesh being non manifold, if yes - it'd be great to update the docs, would be happy to help with that.

@fakufaku
Copy link
Collaborator

Is there a definition of the mesh being manifold ? I am not familiar with the term. My guess would be that the code is not super friendly to weird mesh... 😅 But as long as the number of crosses is consistent, I think it should work.

I would like to try the example you provided but I am not sure which package is stl imported from. I have tried pip install stl but it seems to be a different package.

@thejasvibr
Copy link
Author

thejasvibr commented Jan 30, 2022

Neither was I familiar with the term manifold until a couple of weeks ago (hadn't dealt with 3D models unitl then) :p. Other software (meshlab, meshmixer, etc) kept throwing up errors if the mesh was not manifold etc, and so thought it may be of relevance...

The code I used to run the STL room simulation is based on the room_from_stl.py from the examples directory. The STL package is called 'numpy-stl' (pip install numpy-stl), and the import is import stl

@fakufaku
Copy link
Collaborator

Thanks! Indeed numpy-stl it was, I even wrote that example myself 😅

Would you also share the fixed version of the mesh for me to try ?

@fakufaku
Copy link
Collaborator

fakufaku commented Jan 31, 2022

Now I am not sure anymore. Which file did you share in the repo ? The one before or after pymeshfix ?

I tried a little bit more and it seems that the problem could be fixed by playing with the parameters of the is_inside method.
There are two parameters that can be changed.

  • eps: This is the precision constant used for the floating point operations.
  • room_isinside_max_iter: This is the number of random points used to determine if the points are in the room. If you increase this number the number of failure should decrease. Maybe try to set to 40
pra.libroom.set_eps(1e-5)  # for example
pra.constants.set("room_isinside_max_iter", 40)

Please let me know how that goes!

BTW, I see in the plot that the file data/sliced_small_OC_cleaned.stl still has some normals pointing inward.

PS.
After trying myself, fixing the mesh with pymeshfix was more successful than tuning the parameters above...

@thejasvibr
Copy link
Author

Now I am not sure anymore. Which file did you share in the repo ? The one before or after pymeshfix ?

The shared files (stl and ply) are the 'raw' ones (sliced_small_OC_cleaned), that have the complex geometry. I did not upload the 'fixed' mesh.

I tried a little bit more and it seems that the problem could be fixed by playing with the parameters of the is_inside method. There are two parameters that can be changed.

* `eps`: This is the precision constant used for the floating point operations.

* `room_isinside_max_iter`: This is the number of random points used to determine if the points are in the room. If you increase this number the number of failure should decrease. Maybe try to set to `40`
pra.libroom.set_eps(1e-5)  # for example
pra.constants.set("room_isinside_max_iter", 40)

Please let me know how that goes!

The source must be added inside the room error still pops up even after changing the eps and max_iter arguments (tried max_iter of upto a 100 and eps of upto 1e-3).

BTW, I see in the plot that the file data/sliced_small_OC_cleaned.stl still has some normals pointing inward.

Which package did you visualise the normals with? I checked out the sliced_small_OC_cleaned.stl scan once more in Blender, the normals that seem to point inwards are from the faces behind the actual face forming the cave wall. (the normals of the face behind stick through into the cave inside, at least in Blender). The screenshots below will hopefully illustrate what I described

oc_normals_1

View inside the cave. Black marking indicates faces of interest. The faces are red - meaning they face inside the volume. The normals (light blue lines) however seem to be pointing out.

oc_normals_2

Same view as above, but with one face deleted. The face was enclosing a volume. Blue colour indicates a face pointing outwards. The normals that 'stick out' are from the inside of the volume.

PS. After trying myself, fixing the mesh with pymeshfix was more successful than tuning the parameters above...

Glad to hear there's consistency here. The main concern is that pymeshfix altered an already simplified mesh geometry, making the sound simulation results that much further from the original.

Long reply, but I guess in summary: had no luck with changing the parameters and perhaps the enclosed volumes are causing issues?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants