Skip to content
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

Split floatX tests into their own CI job #5626

Closed
michaelosthege opened this issue Mar 19, 2022 · 3 comments · Fixed by #5630
Closed

Split floatX tests into their own CI job #5626

michaelosthege opened this issue Mar 19, 2022 · 3 comments · Fixed by #5630

Comments

@michaelosthege
Copy link
Member

Currently our CI has a floatX job matrix on Ubuntu and Windows, running each set of tests with both float32 and float64.

But there are just a handful of tests where floatX actually makes a difference.
Only these tests should run with a floatX job matrix, thereby dramatically reducing total CI runtime.

The following screenshot is an incomplete list of candidates:

image

@michaelosthege michaelosthege changed the title Split floatX tests into their CI job Split floatX tests into their own CI job Mar 19, 2022
@ricardoV94
Copy link
Member

I would definitely remove test_distributions from float32 tests. Those xfails are not informative of anything because they arise from comparing with scipy methods that are always computed on float64

michaelosthege added a commit to michaelosthege/pymc that referenced this issue Mar 20, 2022
michaelosthege added a commit to michaelosthege/pymc that referenced this issue Mar 20, 2022
michaelosthege added a commit to michaelosthege/pymc that referenced this issue Mar 20, 2022
michaelosthege added a commit to michaelosthege/pymc that referenced this issue Mar 20, 2022
@ricardoV94
Copy link
Member

Some tests I think make sense parametrizing with both floatX:

def test_mixture_dtype(self):

def check_dtype(self):
assert pm.Constant.dist(2**4).dtype == "int8"
assert pm.Constant.dist(2**16).dtype == "int32"
assert pm.Constant.dist(2**32).dtype == "int64"
assert pm.Constant.dist(2.0).dtype == aesara.config.floatX

def test_custom_dist_sum_stat(self):

def test_custom_dist_sum_stat_scalar(self):

Probably pm.floatX helper is also tested directly, hopefully with the flags as a parametrization or something.

There should be also a DensityDist test that checks the dtype follows floatX

This list is pretty ad hoc though

@michaelosthege
Copy link
Member Author

michaelosthege commented Mar 20, 2022

Running this on Windows indicates a previously unknown... behavior.

    @pytest.mark.parametrize("floatX", ["float32", "float64"])
    def test_dtype(self, floatX):
        with aesara.change_flags(floatX=floatX):
            assert pm.Constant.dist(2**4).dtype == "int8"
            assert pm.Constant.dist(2**16).dtype == "int32"
>           assert pm.Constant.dist(2**32).dtype == "int64"
E           AssertionError: assert 'uint64' == 'int64'
E             - int64
E             + uint64
E             ? 

Also for -(2**32) is becomes uint64.

What should I do about it?
Looks like an Aesara bug. I opened aesara-devs/aesara#871 and will reference in a conditional XFAIL.

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

Successfully merging a pull request may close this issue.

2 participants