org-arl / SignalAnalysis.jl

Signal analysis toolbox for Julia
MIT License
41 stars 10 forks source link

Support for 2D steering #8

Closed prasadtiru closed 4 years ago

prasadtiru commented 4 years ago

Added the following features:

  1. Support for 2D steering.
mchitre commented 4 years ago

In the spirit of avoiding duplicate functionality across the Julia ecosystem, would one of these not suffice?

...for the functionality in 2 and 3.

baggepinnen commented 4 years ago

LinRange2D can be recreated using broadcast like so

julia> reduce(vcat, hcat.(LinRange(0,1,4)', LinRange(2,3,5)))
20×2 Array{Float64,2}:
 0.0       2.0
 0.0       2.25
 0.0       2.5
 0.0       2.75
 0.0       3.0
 0.333333  2.0
 0.333333  2.25
 0.333333  2.5
 0.333333  2.75
 0.333333  3.0
 0.666667  2.0
 0.666667  2.25
 0.666667  2.5
 0.666667  2.75
 0.666667  3.0
 1.0       2.0
 1.0       2.25
 1.0       2.5
 1.0       2.75
 1.0       3.0
prasadtiru commented 4 years ago

prof @mchitre : I've now removed both LinRange2D and meshgrid from this file since these were just convenient functions. These can be implemented by anyone utilizing existing libraries as you pointed out. I've also updated the documentation accordingly.

@baggepinnen Thanks. That works!

mchitre commented 4 years ago

@prasadtiru could you add some test cases for this please? Also, would be good to note in the doc that θ can be 1D or 2D angles, and not replace with 1D by 2D.

prasadtiru commented 4 years ago

I've now added tests for testing the 2D steering code and fixed the documentation as suggested.

baggepinnen commented 4 years ago

The code contains a lot of if statements to handle the cases vector vs. matrix. It would perhaps be cleaner and more idiomatic julia to have two separate methods

steering(rxpos, c, θ::AbstractVector)
steering(rxpos, c, θ::AbstractMatrix)

and get rid of all if statements.

Note also that

ndims(θ) == 1 ? ndir = length(θ) : ndir = size(θ, 1)

is the same as

ndir = size(θ, 1)
prasadtiru commented 4 years ago

I've added a separate method now as suggested to deal with cases vector vs matrix.