-
Notifications
You must be signed in to change notification settings - Fork 4
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
Fix PETSc tests in CI #54
Comments
Yeah I can't even get this to run locally, but that's probably my M1 not your tests!
|
Should we be worried about this?
Specifically the |
Hey Matt. On the first point: this test and all the other PETSc ones run fine for me locally. Every time I add a new test I run through all the previous ones: I also created a nopetsc conda environment so I can specifically test the skipping of the tests (so I don't make the same import mistakes as earlier). I suspect what's failing here is that PETSc is not being installed with the full configuration with the Github action. Just checked out the action: we need to add some environment variables to make sure that when pip does its thing, it installs PETSc properly configured, i.e. with support for complex numbers and various auxiliary libraries like scalapack and mumps. Specifically, we need to add this line to the installation action: EDIT: wait, I got mixed up between your local trace stack and the Github action. I am surprised that all the tests work, considering that the PETSc being installed does not have complex number support. On the Github stacktrace from the action you linked, it seems that the pytest coverage code is failing, no?
|
On the second question: yes, this is okay. Some of the tests will run PETSc with brigade_size = world_size, i.e. will all available ranks dedicated to the parallel solution of a single (k,w) point. In that case, there is no need to chunk_jobs, so it gives the warning. Looking to the two lines above the one you mentioned, there is the line that says "running with one brigade only". So Chunking with Comm_World_Size = 1 is expected behaviour.
|
I just checked and if I run the tests locally with the command you gave above |
I wonder if cov does not play nicely with mpi testing. If I run without mpiexec then it skips the tests just fine and the full command does not error out, it is able to write coverage.xml and everything. |
Yup, looks like this is an active issue: pytest-dev/pytest-cov#237. |
@Chiffafox Ok lots to unpack here: First, I do include
The
I'm honestly not sure! 😁
That's really interesting... guess as you say though this is an open issue. For now I guess we just cannot upload those tests.
Sounds good. Anyway, if you have the time feel free to modify the CI file, push to master to trigger the runs. I might run out of compute time soon though while the repo is still private, so just try not to overdo it! |
Crazy question: can we run PETSc with |
Ah okay I missed the config export, sorry! 😃 I am following a fix with suggested by someone in the issue now by using a setup.cfg for the coverage -- let's see if it work,s and if it does, we can think about how to integrate it here. What I gathered from reading the issue and linked things is that basically, the problem is that pytest is not MPI-aware, so at some point not all cores are finished writing the reports (or the reports are slightly different) and pytest fails because of an sqlite read error, trying to combine the reports.
We certainly can, but the point of most of the tests is to actually test PETSc running in the parallel (and double-parallel, system-saving, etc.) regime. At WORLD_SIZE=1 many of the tests would become redundant and won't test the functionality. Again, the PETSc test runs okay, like you said above -- it's only the pytest coverage that fails in MPI mode at the very end step of combining reports. |
Okay, looks like the fix offered in that issue works for me locally. The fix is to create a setup.cfg file with
and then add --cov-config=setup.cfg to the list of variables when running pytest. Namely the new command would be
I will run this locally for all the tests to make sure it works. Then I was thinking of adding the petsc_test_setup.cfg file to the _tests/petsc folder and amending the pytest command on the Actions. How does that sound? |
Okay, I think I found a way to do it. It seems to work for me locally. The serial part is unchanged, but to run the MPI and PETSc ones I will use the command
and
respectively. This is running exactly the same thing as pytest --cov, except that it allows the different MPI ranks to explicitly write to different .coverage.rank###.timestamp files, which is threadsafe and raises no sqlite problems. Afterwards, I will run an action of combining the files and making the final xml report
which I think should be easily uploadable by codecov. How does this sound? I've got a branch with this and warning ignore for numpy and renaming of spectrum that I will open a PR to momentarily. |
This is pretty sick, yeah I'll check out the PR. Thank you! |
@Chiffafox I think we're done with this no? |
Yessir! Closing the issue now with the acceptance of PR #55. |
@Chiffafox looks like I've got almost everything figure out on
ubuntu-latest
. However, the PETSc tests seem to be finishing with 100% success, but at the end they kinda randomly fail.Would you mind verifying that these work locally for you? Here's the action in question: https://github.com/x94carbone/GGCE/actions/runs/3246363273/jobs/5325016321.
The text was updated successfully, but these errors were encountered: