Add xdist to image compare

[cvanelteren]

This is a continuation of #190.

I added some supporting code to conftest to allow our tests to run in parallel. In addition it adds a custom html to show the results and is compliant with our approach of only keeping the failed images.

I also sorted the dependencies alphabetically in environment.yml to reduce my OCD ;-)

[codecov]

Codecov Report

Attention: Patch coverage is 37.98883% with 333 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
ultraplot/tests/mpl_plugin/reporting.py	10.17%	150 Missing ⚠️
ultraplot/tests/mpl_plugin/utils.py	26.15%	48 Missing ⚠️
ultraplot/tests/mpl_plugin/cleanup.py	24.59%	46 Missing ⚠️
ultraplot/config.py	60.74%	30 Missing and 12 partials ⚠️
ultraplot/tests/conftest.py	51.21%	19 Missing and 1 partial ⚠️
ultraplot/tests/mpl_plugin/core.py	60.86%	15 Missing and 3 partials ⚠️
ultraplot/tests/mpl_plugin/progress.py	76.31%	8 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[cvanelteren]

On the GHA we are running automatically with two workers, not sure if we can force it to use more -- it's not much but it's something I guess. Locally it runs much faster.

[beckermr]

For the RNGs, I wonder if something in pytest is getting the fixtures confused. Maybe we move to making the RNGs directly within each test?

[cvanelteren]

For the RNGs, I wonder if something in pytest is getting the fixtures confused. Maybe we move to making the RNGs directly within each test?

I am creating a new rng for each thread now.
I just checked our context manager is not thread safe. Will add it.

[cvanelteren]

I am still expecting the tests will fail when comparing with main. Race conditions are gone locally 🥳 and the test suite is more robust. I may wanna move the plugin to another repo outside the main repository as it contains a lot more code than I was expecting when I started working on it.

[beckermr]

I think the new rng test fixture is using a different effective seed. We should figure that out.

[cvanelteren]

I think the new rng test fixture is using a different effective seed. We should figure that out.

Should be the same no? See

UltraPlot/ultraplot/tests/conftest.py

Line 11 in f0bd292

seed = 51423

[beckermr]

I think this line

return np.random.Generator(np.random.PCG64(SEED))

may need to be

return np.random.default_rng(seed=SEED)

The image comparison tests are failing right now because the data in them is different. So something broke.

[cvanelteren]

I believe these are the same for numpy, i.e. the default rng is PCG64 but can change it.

[beckermr]

Ohhhh I thought it was the MT one. Hrrmmmmm ok. We need to figure out why the tests now differ.

[cvanelteren]

I am suspecting that the seed was not properly resetting in its current implementation which makes comparison hard as we don't know when a thread runs a particular test (the racing condition).

I can confirm that generating the baseline from the PR and then running a comparison on that data will always match which indicates that the racing condition is not the problem, but the comparison with main will still fail.

I think what we could do is first merge a PR that ensures the numpy rng is set properly on main and then re-compare this with this PR, what do you think?

[cvanelteren]

I will do that now -- time permitting. At least this PR is working with xdist now nicely which is great and the speed ups are very nice for local development 💯

[cvanelteren]

I am going ahead an merging this as the issue is again related to the rng generation. I will confirm with the merged results offline is the issue still persists.

[cvanelteren]

Found no issues other than then network layout not properly passing the seed, see #256