nest-simulator icon indicating copy to clipboard operation
nest-simulator copied to clipboard

Add more minimal CI test

Open terhorstd opened this issue 1 year ago • 4 comments

This change adds one more build to the CI/CD chain to test for the Boost dependency.

terhorstd avatar Jul 22 '24 09:07 terhorstd

It seems difficult to uninstall the boost headers due to dependencies of other packages. Important would be a proof that the CI fails when #3258 is reverted. Do you have an idea how to best go about that? :thinking:

terhorstd avatar Aug 26 '24 13:08 terhorstd

Pull request automatically marked stale!

github-actions[bot] avatar Oct 26 '24 08:10 github-actions[bot]

This seems good to go once the break is reverted. With the commit https://github.com/nest/nest-simulator/pull/3257/commits/243449d35f5c154a90fcf54e5441380d94d34a94 correctly being flagged broken by the CI:

Error: /home/runner/work/nest-simulator/nest-simulator/libnestutil/iterator_pair.h:26:10: fatal error: boost/iterator/iterator_adaptor.hpp: No such file or directory
   26 | #include <boost/iterator/iterator_adaptor.hpp>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

:warning: This is however only partial protection as we have seen in https://github.com/nest/nest-simulator/pull/3257/commits/ff5dbc3161c8c892b5e5553d5b9f500e5ca6c516 not being flagged. This additional build can only protect against boost-includes in parts of the code that is not include-guarded by a combination of other dependencies (such as GSL in this case). To protect against this we would need to duplicate the whole test matrix. :zap:

@gtrensch, @lekshmideepu, could you have another look?

terhorstd avatar Feb 28 '25 13:02 terhorstd

Pull request automatically marked stale!

github-actions[bot] avatar Apr 30 '25 08:04 github-actions[bot]

Improved CI runs additionally revealed a spurious call to mpirun in case of -Dwith-mpi=OFF somewhere in the pytest suite (now becoming visible since mpi is not installed anymore on the runner):

 ________________ ERROR at setup of test_pairwise_bernoulli[12] _________________
[gw0] linux -- Python 3.10.18 /opt/hostedtoolcache/Python/3.10.18/x64/bin/python3.10

    @pytest.fixture(scope="session")
    def subprocess_compatible_mpi():
        """Until at least OpenMPI 4.1.6, the following fails due to a bug in OpenMPI, from 5.0.7 is definitely safe."""
    
>       res = subprocess.run(["mpirun", "-np", "1", "echo"])

terhorstd avatar Jul 11 '25 17:07 terhorstd

@gtrensch, the original boost issue is now proven to be caught. I updated the description above. The two commits also show again, that multiple #ifdef may hide the bug from our small build matrix.

The erroneously failing MPI test during non-MPI builds still needs to be fixed.

Thanks for your comments and discussion on this. It helped and improved our understanding of the matter a lot.

terhorstd avatar Jul 25 '25 10:07 terhorstd

@terhorstd, all builds and checks have passed! Depending on CMake option '-Dwith-mpi=on/off' and the availability of 'mpi4py', MPI tests are executed or skipped correctly.

gtrensch avatar Jul 28 '25 15:07 gtrensch

Wow, thanks @gtrensch for fixing this :clap: I agree too your comment that most of the try…except…if… parts are problematic. I would have expected the @unittest.skipIf(not HAVE_MPI, "NEST was compiled without MPI") solution would work in most places, but I see that it would need some deeper work to get that sorted out. I'd opt for putting this in for now and opening a separate issue for looking into a better solution for the HAVE_MPI in the test suite.

terhorstd avatar Jul 28 '25 17:07 terhorstd

I cannot assign myself as reviewer, but agree :+1: to your changes to fix the downstream matter. They are quite orthogonal, so @gtrensch, for the remaining changes should be unproblematic for you to stay reviewer for this PR. As soon as we have both thumbs up again we can merge.

terhorstd avatar Jul 28 '25 17:07 terhorstd

@terhorstd, many thanks for your kind comments. I agree completely. The test suite should handle this at a higher level.

gtrensch avatar Jul 30 '25 10:07 gtrensch

@lekshmideepu Ping!

heplesser avatar Aug 07 '25 13:08 heplesser

Thanks for the very enlightening process and inspiring reviews :+1:

terhorstd avatar Aug 15 '25 07:08 terhorstd