Conversation
|
@oxinabox @fredrikekre Could some of you please review this PR, so we can move forward with JuliaDiff/ForwardDiff.jl#589? Thank you! |
|
@mateuszbaran @mohamed82008 Could you pls take a look? |
src/DiffTests.jl
Outdated
| hilbert_matrix(::Type{T}, n::Integer) where T<:Real = | ||
| [convert(T, inv(i + j - 1)) for i in 1:n, j in 1:n] | ||
| hilbert_matrix(x::VecOrMat) = hilbert_matrix(Float64, size(x, 1)) |
There was a problem hiding this comment.
This function doesn't seem to be used anywhere in this PR.
There was a problem hiding this comment.
This function doesn't seem to be used anywhere in this PR.
Yes, it doesn't. I was undecided which constant matrix to use, it looked like with this one the errors are higher (it is beyond my capability to track and possibly fix the source of that errors).
I can remove, if it is critical, but maybe it is better to keep it, anticipating the future extension of the test coverage.
There was a problem hiding this comment.
I generally think keeping unused code in the main branch should be avoided. Everything else looks fine though in case someone with merge rights cares about my opinion 🙂 .
There was a problem hiding this comment.
I agree, this should be removed before we can merge
|
@oxinabox @fredrikekre gentle ping to anyone with PR merge rights :) Please help me to reduce the stack of dependent PRs that I have accumulated :) |
|
@oxinabox Gentle ping :) I have removed the unused test function; also I have added the sparse variants of the new test functions. |
|
@oxinabox Thanks! Can you tag a new release? |
3 participants
The PR adds
f(x) = A*xandf(x) = A\xfunctions, whereAis a constant matrix, andxis either a vector or a matrix.For vector arguments, the new
VECTOR_TO_VECTOR_FUNCSlist of test functions is introduced.The tested
Amatrices are: diagonal, dense, lower and upper triangular. The last three matrix types are represented by the Lehmer matrix.In particular, the
A\xfunctions are useful to test the autodiff of multivariate normal distribution.This PR would be required to properly test JuliaDiff/ForwardDiff.jl#589 (so when/if merged, the version number of DiffTools.jl has to be updated).