Value-based tests for log-likelihoods#728
Merged
MichaelClerx merged 12 commits intomasterfrom Feb 22, 2019
Merged
Conversation
- was previously pulling out the wrong sigma and rho for multi output problems
- Added multi-output test for ARMA11 - Corrected parameter referencing in ARMA11 code - Corrected docs for ARMA11 code
- The sensitivities were previously not correct for multi-output problems - Added multi-output value-based tests for multi-output problems - Corrected mistake with constant_model forward sensitivities (it should be a diagonal matrix rather than one with increasing diagonals) - Corrected errors with sensitivities for Gaussian log-likelihood for multiple output problems: formerly we were returning an array of theta derivatives followed by a single sigma derivative. This is incorrect as there are three sigma parameters - one for each output
- Added value-based tests for Gaussian with known sigma
ben18785
changed the title
- constant model test was wrong previously - corrected incorrect derivative-based tests for the constant model
Codecov Report
@@ Coverage Diff @@
## master #728 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 50 50
Lines 4622 4628 +6
=====================================
+ Hits 4622 4628 +6
Continue to review full report at Codecov.
|
MichaelClerx
requested changes
Feb 19, 2019
Member
MichaelClerx
left a comment
MichaelClerx
left a comment
There was a problem hiding this comment.
Hi Ben, Thanks very much! Some tiny suggestions and then happy to merge
- tweak to docstring for constant model - tweak to derivative function to simplify for constant model - tweak to test comments for error measures to reflect changes to the derivative function for the constant model
Collaborator
Author
|
@MichaelClerx Can we merge this please if you're happy with my changes? |
MichaelClerx
approved these changes
Feb 22, 2019
MichaelClerx
added a commit
that referenced
this pull request
Feb 22, 2019
Value-based tests for log-likelihoods
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In this checking of the tests, I (somewhat disconcertingly) found about a dozen errors that would have been picked up if we were doing value-based tests of log-likelihoods. The errors were on the following:
Covers #719