Closed hkershaw-brown closed 1 month ago
The code is good, allows you to easily use the test_x subroutines in the various distribution modules and makes it more obvious when the checks fail.
note the pass/fail conditions for the matlab tests do no match the comments for gamma and beta. The comment says absolute value, the test is not on the absolute value.
With regards to this comment above that you made in the PR, I think the comments do match the code. The comments say that the "Absolute value of differences should be less than 1e-15", and the conditionals are if(abs(maxval(pdf_diff)) < 1e-15_r8 .and. abs(maxval(cdf_diff)) < 1e-15_r8)
which is using the absoulte value function.
I'm seeing the same results with regards to getting a bunch of "inv_cdf Failed to converge for quantile" messages for normal_distribution, but I am using gfortran instead of Intel.
I'm also getting one of these messages from inv_cdf in the results of beta_test:
inv_cdf Failed to converge for quantile 0.0000000000000000
I can't really tell if this problematic or not for a quantile of 0, but I think we are probably good to go ahead and merge this once I get your thoughts on this @hkershaw-brown
Hi Marlee, thanks for looking at this, I'm leaving this pull request hanging out at the moment. It was an attempt to break up this mega commit from Jeff: https://github.com/NCAR/DART/commit/5b3850fb1a08fff0ebce3f95283fef29de431bdc
There are some problems with the QCEFF, so I don't want to dump these tests into a release at the moment.
FYI "the comments do match the code" because I changed the code. Anyway sorry this is such a round-about way of developing.
Description:
Adds developer tests for normal_dist beta_dist gamma_dist
from https://github.com/NCAR/DART/commit/5b3850fb1a08fff0ebce3f95283fef29de431bdc
note no bnrh test since there is no test_bnrh in bnrh_distribution_mod.f90
note the pass/fail conditions for the matlab tests do no match the comments for gamma and beta. The comment says absolute value, the test is not on the absolute value.
Fixes issue
This pull request does not fix any of the failing tests.
See #717 for discussion of beta failing tests. Normal distribution has
failed to converge
on ifort (IFORT) 2021.10.0 20230609 DerechoTypes of changes
Documentation changes needed?
Tests
Please describe any tests you ran to verify your changes.
Checklist for merging
Checklist for release
Testing Datasets