-
Notifications
You must be signed in to change notification settings - Fork 145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Harnesses for distribution tests #729
Conversation
This is failing GNU Fortran (MacPorts gcc13 13.2.0_4+stdlib_flag) 13.2.0 inv_cdf Failed to converge for quantile 0.0000000000000000 ---------------------------- max difference in inversion is 7.8846335608728112E-006 max difference should be less than 1e-14
Note no test_bnrh subroutine exists
used to create test_X_dist directories
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.
assimilation_code/modules/assimilation/beta_distribution_mod.f90
Outdated
Show resolved
Hide resolved
assimilation_code/modules/assimilation/gamma_distribution_mod.f90
Outdated
Show resolved
Hide resolved
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.
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 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: |
Hi Marlee, thanks for looking at this, I'm leaving this pull request hanging out at the moment. 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good still, and the tests run successfully (although reporting some failures as expected)
Description:
Adds developer tests for
normal_dist
beta_dist
gamma_dist
from 5b3850f
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.fixedFixes 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
onifort (IFORT) 2021.10.0 20230609 Derecho
Types of changes
Documentation changes needed?
Tests
Please describe any tests you ran to verify your changes.
Checklist for merging
Checklist for release
Testing Datasets