distributional icon indicating copy to clipboard operation
distributional copied to clipboard

add more extensive tests

Open venpopov opened this issue 3 months ago • 2 comments

While we figure out what the best approach for #101 is, I thought it might be good to extract some of the tests I wrote for it in a separate PR that could be reviewed more easily and implemented. I was thinking about this, because of your comment here. I hadn't realized that the implementation of nested distributions broke some functions such as quantile, generate, mean and covariance, because they use x[["transform"]]. This is because all the tests passed, but there are no tests for the behavior of these functions for nested transformations.

While trying to fix those for the PR, I realized that mean.transformed and covariance.transformed are alrady broken (#102), and I noted a problem with quantile a few days ago (#100). It would be good to have tests for those as well, to make sure that changes like those introduced by #101 or variations of it work with all of the package functionality. So here's my proposal:

Add methods eval_transform(dist, at), eval_inverse(dist, at) and eval_deriv(dist, at)

In the current master branch, these will be just wrappers for x[["transform"]], x[['inverse']] and numDeriv... But in the future, any changes to how the transform, inverse and deriv functions are stored will have to change these methods. This way the change needs to happen only in one place, whereas all functions like density.transformed, quantile.transformed, etc, while be the same. This should make code maintenance easier, and allow for more robust tests as well

add the tests from #101 that inverses and derivatives are calculated directly

there are some tests in the master branch, but again just for simple transformations, and would be good to have the extensive tests I wrote there

add more tests for quantile, generate, mean and covariance

the only test for mean and covariance of transformed distribution is for a longnormal with mean 0 and sd=0.25, and there the results are close enough, but as I noted in #102, the results are quite incorrect for other parameter values. I know these are supposed to be based on numeric approximation, but the error is quite substantial. There also no tests for nested transforms.

Let me know what you think and I can put together a PR. I would really love to be able to use the package with more confidence and I think this will also help catch more problems during development.

venpopov avatar Apr 04 '24 16:04 venpopov