Skip to content

Systematized Distribution testing overlooks Theano arguments #4001

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

Closed
brandonwillard opened this issue Jul 5, 2020 · 2 comments
Closed

Systematized Distribution testing overlooks Theano arguments #4001

brandonwillard opened this issue Jul 5, 2020 · 2 comments

Comments

@brandonwillard
Copy link
Contributor

brandonwillard commented Jul 5, 2020

I noticed that our testing framework doesn't perform tests using Theano arguments (not within our systematized tests—like TestMatchesScipy—at least).

This seems like a pretty big oversight, and appears to have let #3999 go unchecked.

In general, I would say that Theano arguments make a much better default than NumPy, so perhaps we should consider changing our test framework to use them.

Originally posted by @brandonwillard in #4000 (comment)

@ricardoV94
Copy link
Member

ricardoV94 commented Jun 4, 2021

@brandonwillard I think the main check_logp and check_logcdf (the later after #4736) in TestMatchesScipy are now testing with aesara arguments via shared variables. Should we close this one?

@brandonwillard
Copy link
Contributor Author

brandonwillard commented Jun 4, 2021

If we're talking about v4, then the concerns expressed in this issue aren't really the same/present. This issue was prompted by errors in our our mixed use of symbolic/non-symbolic values (i.e. special handling for values that could be either type).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants