JuliaApproximation / MultivariateOrthogonalPolynomials.jl

Supports approximating functions and solving differential equations on various higher dimensional domains such as disks and triangles
Other
17 stars 5 forks source link

Simplify Zernike Jacobi matrices + bugfixes #123

Closed TSGut closed 2 years ago

TSGut commented 2 years ago

This makes the fixes made in https://github.com/JuliaMatrices/InfiniteLinearAlgebra.jl/pull/103 also work for the Zernike Jacobi matrices.

codecov[bot] commented 2 years ago

Codecov Report

Merging #123 (2b3012d) into master (83421e6) will decrease coverage by 0.08%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #123      +/-   ##
==========================================
- Coverage   96.23%   96.15%   -0.09%     
==========================================
  Files           4        4              
  Lines         718      702      -16     
==========================================
- Hits          691      675      -16     
  Misses         27       27              
Impacted Files Coverage Δ
src/disk.jl 98.08% <100.00%> (-0.14%) :arrow_down:

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 83421e6...2b3012d. Read the comment docs.

TSGut commented 2 years ago

I think this covers all the actions one may want to do with these operators for now. Ready to merge.

TSGut commented 2 years ago

@dlfivefifty So what's the mechanism allowing for the use of Z \ (x .* Z) to call the jacobi matrices? That is still missing to make this implementation harmonize properly with the rest of the package. Tried to see what happens for the Triangle and Xu Disk by debug entering but the calls seemed more complicated than I remember it from e.g. the fractional Laplacian implementation I did.

dlfivefifty commented 2 years ago

That should just work provided you overload jacobimatrix(::Val{1}, ...):

https://github.com/JuliaApproximation/HarmonicOrthogonalPolynomials.jl/blob/869f0313a180a7352f0bb8b554399d7b863aef83/src/multivariateops.jl#L37

TSGut commented 2 years ago

I figured out why it wasn't working. Basically, the stuff you linked at one point requires copy(jacobimatrix(Val(1),Z)) to work but that wasn't working because copy wasn't defined for the type I made to hide the excessive type printout so it defaulted to something for arbitrary block arrays which doesn't work on the infinite case.

I think I fixed it by overloading copy for the bands type.

TSGut commented 2 years ago

@dlfivefifty This is ready to merge I think.

TSGut commented 2 years ago

By comparing with the Triangle code, I've decided it's better to not have the custom type for the Jacobi matrices for now, especially since it just leads to harder to read and debug code and actually slows things down for an enduser.

I've also removed the BlockHcat call. As a result, the tests I implemented here won't pass until the addition bug mentioned here https://github.com/JuliaApproximation/MultivariateOrthogonalPolynomials.jl/pull/123#discussion_r862328705 has been resolved.

TSGut commented 2 years ago

@dlfivefifty This is ready to merge and can now do polynomial multiplication operators. I'm pretty sure the coverage reduction is an artifact due to a reduction in lines of code as the patch diff is 100%.