AbstractGPs.jl
AbstractGPs.jl copied to clipboard
Use non scalar method to compute logdet for cholesky and relax compat for PDMat
This enables the computation of dtc
on GPU
Will be creating an issue in https://github.com/JuliaLang/julia
Could you add a test to make sure that logdet_ == logdet
?
Also why the relaxation on the PDMats Compat?
Also why the relaxation on the PDMats Compat?
I encounter this without the relaxation
(WilliamsGPR) pkg> dev AbstractGPs
Resolving package versions...
ERROR: Unsatisfiable requirements detected for package ElasticPDMats [2904ab23]:
ElasticPDMats [2904ab23] log:
├─possible versions are: 0.1.0-0.2.2 or uninstalled
├─restricted by compatibility requirements with GaussianProcesses [891a1506] to versions: 0.2.0-0.2.2
│ └─GaussianProcesses [891a1506] log:
│ ├─possible versions are: 0.8.0-0.12.4 or uninstalled
│ └─restricted to versions 0.12 by WilliamsGPR [c957e16a], leaving only versions 0.12.0-0.12.4
│ └─WilliamsGPR [c957e16a] log:
│ ├─possible versions are: 0.2.0 or uninstalled
│ └─WilliamsGPR [c957e16a] is fixed to version 0.2.0
└─restricted by compatibility requirements with PDMats [90014a1f] to versions: uninstalled — no versions left
└─PDMats [90014a1f] log:
├─possible versions are: 0.9.0-0.11.16 or uninstalled
└─restricted to versions 0.11 by AbstractGPs [99985d1d], leaving only versions 0.11.0-0.11.16
└─AbstractGPs [99985d1d] log:
├─possible versions are: 0.5.14 or uninstalled
├─restricted to versions 0.5 by WilliamsGPR [c957e16a], leaving only versions 0.5.14
│ └─WilliamsGPR [c957e16a] log: see above
└─AbstractGPs [99985d1d] is fixed to version 0.5.14
Codecov Report
Merging #329 (8aae82a) into master (6ee8549) will increase coverage by
0.00%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #329 +/- ##
=======================================
Coverage 97.62% 97.63%
=======================================
Files 10 10
Lines 379 380 +1
=======================================
+ Hits 370 371 +1
Misses 9 9
Impacted Files | Coverage Δ | |
---|---|---|
src/sparse_approximations.jl | 100.00% <100.00%> (ø) |
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 6ee8549...8aae82a. Read the comment docs.
I don't understand the changes in this PR, can you explain the motivation?
The compat issues seem to be caused by ElasticPDMats/GaussianProcesses/WilliamsGPR. I don't remember the details but there was surely a reason for why we dropped support for PDMata 0.10. So I think it would be better to fix the other problematic packages but not to reintroduce the PDMats 0.10 compat entry.
The change to _logdet
is also unclear to me as it breaks the package in its current form. I guess you maybe forgot to define _logdet(x) = logdet(x)
fallback? In any case, I don't think such an internal method for Cholesky
should be introduced here. The logdet
definition for Cholesky
in base (https://github.com/JuliaLang/julia/blob/46a6f2230d9d05f0bb26ca5d241a7316993c10a8/stdlib/LinearAlgebra/src/cholesky.jl#L669) is fine, and I assume GPU support should be added in CUDA.jl or similar packages, probably by defining something like logdet(::Cholesky{T,CuMatrix{T}})
(see eg also https://github.com/JuliaGPU/CUDA.jl/issues/110).
I'll close this PR for now. Please reopen it if you intend to address https://github.com/JuliaGaussianProcesses/AbstractGPs.jl/pull/329#issuecomment-1191856948 :slightly_smiling_face: