Clean up HeckeMiscMatrix.jl
#1851
Conversation
| @@ -89,3 +89,6 @@ end | |||
| @deprecate is_power(x::IntegerUnion) is_perfect_power_with_data(x) | |||
| @deprecate is_power(x::QQFieldElem) is_perfect_power_with_data(x) | |||
| @deprecate is_power(x::Rational) is_perfect_power_with_data(x) | |||
|
|
|||
| # To be deprecated in 0.47 | |||
| Array(a::ZZMatrix; S::Type{T} = ZZRingElem) where {T} = Matrix{T}(a) | |||
There was a problem hiding this comment.
Please check whether this is fine.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1851 +/- ##
==========================================
+ Coverage 87.02% 87.38% +0.35%
==========================================
Files 98 97 -1
Lines 35925 35887 -38
==========================================
+ Hits 31265 31360 +95
+ Misses 4660 4527 -133 ☔ View full report in Codecov by Sentry. |
joschmitt
force-pushed
the
js/miscmat
branch
from
3ab4eb6 to
72b6bbf
Compare
joschmitt
force-pushed
the
js/miscmat
branch
from
72b6bbf to
95da750
Compare
| d = one(ZZ) | ||
| for i in 1:nrows(M) | ||
| for j in 1:ncols(M) | ||
| d = lcm!(d, d, denominator(M[i, j])) |
There was a problem hiding this comment.
We should import denominator!(z::ZZRingElem, y::QQFieldElem) from Hecke. And then extend it to accept QQFieldElemOrPtr, and do something like
| d = lcm!(d, d, denominator(M[i, j])) | |
| d2 = denominator!(d2, mat_entry_ptr(M, i, j)) | |
| d = lcm!(d, d, d2) |
where d2 is a second temporary. That's of course still not going to get speed records but at least it'll be a bit less wasteful.
Of course that's not a request for this PR, just musing about this code will browsing past it.
| @@ -476,6 +476,14 @@ end | |||
|
|
|||
| ==(x::ZZRingElem, y::ZZMatrix) = parent(y)(x) == y | |||
|
|
|||
| # Return a positive integer if A[i, j] > b, negative if A[i, j] < b, 0 otherwise | |||
| function compare_index(A::ZZMatrix, i::Int, j::Int, b::ZZRingElem) | |||
There was a problem hiding this comment.
This name is bad (we are not comparing indices). Perhaps we should move it back to Hecke, then it can be renamed there, and in fact reimplemented using mat_entry_ptr plus cmp.
| function prod_diagonal(A::ZZMatrix) | ||
| a = one(ZZ) | ||
| GC.@preserve a A begin | ||
| for i = 1:min(nrows(A),ncols(A)) | ||
| b = mat_entry_ptr(A, i, i) | ||
| mul!(a, a, b) | ||
| end |
There was a problem hiding this comment.
Can't we use this as a more efficient generic implementation with minor tweaks?
No description provided.