Relative Difference Prior Abs fix #1410
Merged
Conversation
This file contains 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
This will allow implementation specific tests for a particular prior.
- we were using abs() which (at least on some systems) converted to integers, giving the wrong result. now using std::abs() - introduce value/derivative_10 function for more concise code
now use INFINITY for second order derivatives, obtained by continuity
split the tests for the different priors in different classes, such that we can have specific tests for each. Currently, only added some for the RDP, including a Lipschitz test and limit for large values.
The RDP test was failing with eps=1.e-3, so now using 1.e-4
|
I think this is done. @Robert-PrescientImaging @epapoutsellis @samdporter. Possibly you could have a look. I'd like to merge this soon though. |
|
I only had time for quick glance but LGTM. |
robbietuk
approved these changes
Apr 27, 2024
|
GitHub actions failure due to unrelated #1411. I'll merge after cleaning up some history. |
use double in neighbourhood calculations
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Changes in this pull request
abs()(leading to integer calculations) withstd::abs()value()andderivative_10functionsepsilon==0) by imposing continuity. This means thatthe derivative now returns 1/(1+gamma) andthe Hessian returnsINFINITY.This PR is backwards incompatible as RDP has changed drastically. However, it is a bug-fix.
Testing performed
Added extra tests based on Mathematica analysis
Related issues
Fixes #1409