Guard VTXWriter use when ADIOS2 is unavailable - #4254
Conversation
|
While you are at it, you may also want to look at the following other places which show a similar issue. This is how we patch it for debian, but there are for sure other ways to handle that (e.g., as you did, moving the https://salsa.debian.org/science-team/fenics/fenics-dolfinx/-/blob/25530d9953e688fa1848ea159c1469f7f89bc3b7/debian/patches/test_adios2_skip.patch |
2685f02 to
c560eb5
Compare
|
Thanks for the pointers. I expanded this beyond the original import-order fix: the ADIOS2 test module now skips when DOLFINx is built without ADIOS2, and the scattering boundary conditions demo guards its VTXWriter output similarly. |
schnellerhase
left a comment
There was a problem hiding this comment.
Can you link #3995 so it auto-closes on merge?
|
Looks good to me too, will help avoiding having to dance around different configurations for the 32-bit architectures. |
| from dolfinx.graph import adjacencylist | ||
| from dolfinx.mesh import CellType, create_mesh, create_unit_cube, create_unit_square | ||
|
|
||
| pytestmark = pytest.mark.skipif(not has_adios2, reason="DOLFINx has been built without ADIOS2") |
There was a problem hiding this comment.
This isn't a good change. If we run test_adios2.py it should adios2, and fail if it can't.
We can have a situation where a change could cause in the CI DOLFINx to (silently) not build with ADIOS2. Changes like this one mean the issue is not picked up and we lose test coverage. In tests, skipping should be explicit.
There was a problem hiding this comment.
ADIOS2 is an optional dependency. The test target pytest should give a suitable set of tests to allow for installation verification for any given configuration. If the we have not compiled with adios2 support a test failure due to a missing import or non-available functionality prohibits this.
Verification of having picked up the desired configuration in a CI run is not limited by this change https://github.com/FEniCS/dolfinx/blob/main/.github/workflows/ccpp.yml#L134.
import_adios2 might not have the right behaviour here, as this will skip a check if adios2 support is enabled and the adios2 python module is not available (skipping silently checks for supported functionality). But that was not altered in this PR.
Skip the ADIOS2 VTXWriter test module when DOLFINx itself is built without ADIOS2 support, while still using the Python adios2 import check for tests that inspect BP output.\n\nAlso guard VTXWriter output in the scattering boundary conditions demo so the demo can continue when ADIOS2-backed VTX output is unavailable.\n\nFixes #3995.\n\nSplit out from #4249 per maintainer request.