kitzeslab / opensoundscape

Open source, scalable software for the analysis of bioacoustic recordings
http://opensoundscape.org
MIT License
134 stars 14 forks source link

Make SPEED_OF_SOUND a class attribute instead of module/global variable #725

Open louisfh opened 1 year ago

louisfh commented 1 year ago

I think instead of declaring the variable SPEED_OF_SOUND in the localization module, we should set it as an attribute of SynchronizedRecorderArray and SpatialEvent. We can have default values of 343 m/s.

I think this would make it easier for people to adjust the speed_of_sound if they are performing localization in another medium (e.g. marine), or want to adjust the SPEED_OF_SOUND based on humidity/temperature/elevation.

sammlapp commented 1 year ago

The reason it’s a module level global variable is that I think it should only be defined in one place. Would get and set functions for the module resolute your concern?

louisfh commented 1 year ago

That's a good point. Functions outside of the 2 classes I mentioned require a speed_of_sound argument (localize, calculate_tdoa_residuals) and its helpful to define a default in one place for those. I guess get and set functions would work.

Alternatively, we could:

The risk with the second approach (which I guess is what you were trying to avoid) is if in the code base we accidentally pass SPEED_OF_SOUND to a method, instead of self.speed_of_sound, or forget to pass a speed_of_sound value and it defaults to SPEED_OF_SOUND.

sammlapp commented 8 months ago

Stepping back, would it make more sense for each SpatialEvent to have a temperature and humidity that are used to calculate speed of sound?

sammlapp commented 5 months ago

(and medium / density, ie air / water)

sammlapp commented 5 months ago

maybe sos should be attribute of SynchronizedRecorderArray object. can use sos function to calculate it for given temp/hum/density