use Symmetric() to get rid of complex eigen() results

[aplavin]

No description provided.

[codecov]

Codecov Report

Merging #68 (fde53e4) into main (97af0fe) will increase coverage by 0.58%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #68      +/-   ##
==========================================
+ Coverage   89.98%   90.56%   +0.58%     
==========================================
  Files          12       12              
  Lines         529      530       +1     
==========================================
+ Hits          476      480       +4     
+ Misses         53       50       -3     

Impacted Files	Coverage Δ
src/bounds/ellipsoid.jl	85.39% <100.00%> (+0.16%)	⬆️
src/proposals/Proposals.jl	93.82% <0.00%> (+1.68%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 29a7cbe...fde53e4. Read the comment docs.

[mileslucas]

Rather than incorporate this here, what if we wrote

decompose(A::AbstractMatrix) = decompose(Symmetric(A))

function decompose(A::Symmetric)
	E = eigen(A)
	#...
end

[aplavin]

I think it makes most sense to have Ellipsoid.A field to have type of Symmetric: covariance matrices are symmetric anyway. Unfortunately, it doesn't work because inplace broadcasting with Symmetric isn't implemented in julia yet - there are several issues, eg JuliaLang/julia#38056. So for now, in this PR, I put the Symmetric wrapper as "high" in the call stack as possible. Shifting it to the decompose() level is also possible, of course. Would you prefer that variant?

[mileslucas]

Thanks for pointing out the mutation issues- I wasn't aware of those. Yes, I prefer to have this at the decompose level, it's not really too different from what you wrote, but I like using the method signatures. If we had a trait system, we could easily switch from decompose(::Symmetric) to something like decompose(::T) where issymmetric(T). FYI, I do have hopes of improving the entire Bounds interface, since each of these bounds is a sample-able (i.e., implement the Distributions.jl API for samplers), which would also make them immutable.

[aplavin]

Ok, changed.
My point was that it mathematically only makes sense to run decompose with Symmetric matrices, same for the Ellipsoid creation and other related functions. Shouldn't need any traits even potentially, I think.

[mileslucas]

Good point, I've noted that in my ideas for the improvements I alluded to.