-
-
Notifications
You must be signed in to change notification settings - Fork 16
evalpoly for matrix polynomials #1163
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
base: master
Are you sure you want to change the base?
Conversation
In addition to julia> evalpoly([1 2; 3 4], (5, 6)) # error without this PR
2×2 Matrix{Int64}:
11 12
18 29
julia> evalpoly([1 2; 3 4], (5, 6.7))
ERROR: InexactError: Int64(6.7)
Stacktrace:
...
[11] evalpoly!(Y::Matrix{Int64}, X::Matrix{Int64}, p::Tuple{Int64, Float64})
@ Main ./REPL[4]:19
julia> evalpoly([1.0 2.0; 3.0 4.0], (2, 3+im, 4))
ERROR: InexactError: Float64(7.0 + 1.0im) |
Argh, this is a PITA to get right for heterogeneous tuples of coefficients, especially for a tuple of dimensionful coefficients. (e.g. suppose that What is a good way to compute the correct element type of the result matrix? For One option would be to just punt on |
Ah now I see some logic for Only accepting Easy to add methods to send your own fancy types to |
I tightened up the type signatures for dispatch to |
This is a little harder to fool, but: julia> evalpoly([1 2; 3 4], Number[5.5, 6])
ERROR: InexactError: Int64(11.5)
julia> evalpoly([1.0 2.0; 3.0 4.0], Number[2, 3, 4+im])
ERROR: InexactError: Float64(4.0 + 1.0im) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1163 +/- ##
==========================================
- Coverage 92.03% 92.02% -0.02%
==========================================
Files 34 35 +1
Lines 15501 15550 +49
==========================================
+ Hits 14267 14310 +43
- Misses 1234 1240 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I think it makes sense to dispatch to |
Co-authored-by: Martin Holters <[email protected]>
Is this good to merge (assuming tests pass)? |
Would it be better to merge this if it is at least addressing some of the issues? |
I have no strong feelings... my examples above invented to break this do seem unlikely to show up in the wild. |
Closes #1161.
Includes a generic out-of-place fallback implementation as well as an in-place
evalpoly!
method; the latter is called by default only forX::StridedMatrix{<:Number}
for now, to be conservative. (e.g. it's much trickier to allocate the in-place buffers correctly for matrices of matrices, which we don't support very well, and I'm not sure this would work well for sparse matrices.) More optimized cases can always be added later.