Turing.jl
Turing.jl copied to clipboard
Fix error TuringOptimExt.jl with information matrix/vcov
I've been using the MLE/MAP estimation functionality (defined here) with some Turing models, and I've come across an error in the informationmatrix
function, extended from StatsBase.
In the function definition, the Hessian of the log likelihood or joint is first calculated on the unconstrained scale. Then that matrix is inverted and returned as the observed Fisher information matrix for the MLE/MAP. StatsBase.vcov
is then extended to basically just be an alias for informationmatrix
.
This seems to be incorrect. According to Wikipedia, the observed information matrix is just the negative of the Hessian (not the inverse), and the variance-covariance matrix is the inverse of the information matrix. This pull request fixes these two spots in the code.
@cpfiffer, since you worked on this code, can you take a look, please?
My pleasure! I've been using Turing a lot in the last couple years, so I'm happy to be able to contribute a little bit. :)
It looks like there are a couple of tests still failing. I can have a look at those next week and update the pull request when they pass.
I've now updated the tests, but they still seem to be failing. Maybe I missed a step somewhere in getting the CI pipeline to run the new test versions?
@smith-garrett I can confirm CI tests are running on the most recent releases of DynamicPPL and related numerical libraries. Maybe take another look?
@yebai Thanks for the update. I think my tests were off, but I think I have fixed them now. The vcov and information matrices now have separate tests based comparing the results to analytical calculations.
@yebai Sorry, there were some silly errors in my last commit. I think I've fixed everything now, as the tests are all passing on my computer. 🤞
Codecov Report
Attention: Patch coverage is 0%
with 2 lines
in your changes are missing coverage. Please review.
Project coverage is 0.00%. Comparing base (
0b56415
) to head (427095b
).
Files | Patch % | Lines |
---|---|---|
ext/TuringOptimExt.jl | 0.00% | 2 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #2167 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 21 21
Lines 1368 1367 -1
======================================
+ Misses 1368 1367 -1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Pull Request Test Coverage Report for Build 8157507510
Details
- 0 of 2 (0.0%) changed or added relevant lines in 1 file are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage remained the same at 0.0%
Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
---|---|---|---|
ext/TuringOptimExt.jl | 0 | 2 | 0.0% |
<!-- | Total: | 0 | 2 |
Totals | |
---|---|
Change from base Build 8140894641: | 0.0% |
Covered Lines: | 0 |
Relevant Lines: | 1367 |
💛 - Coveralls
There are still two failing tests, but they don't seem to have to do with the information matrix/vcov stuff as far as I can tell. I hope I haven't introduced a new error somewhere!
Thanks @smith-garrett -- there are some tests that have a stringent numerical error threshold which fails occasionally due to stochasticity! These have gone away after running the CIs.