SPHinXsys icon indicating copy to clipboard operation
SPHinXsys copied to clipboard

[CI] todo improvements

Open FabienPean-Virtonomy opened this issue 2 years ago • 6 comments

  • [x] Linux #174
  • [x] Windows #174
  • [x] MacOS X #181
  • [x] Documentation #174
  • [ ] Make the CMake install tree and export targets
  • Matrix
    • [ ] Different library linkage shared/static
    • [ ] Different OS versions (Ubuntu 20.04/22.04, Windows 2019/2022, macOS)
  • [ ] Build CMake targets based on commit diff
  • [ ] Cleanup unnecessary CMake scripts
  • [x] Split CI workflow/jobs to reduce waste in case of failure (split cache of dependencies from building files). Item can be discarded after https://github.com/friendlyanon/setup-vcpkg/pull/6 is accepted
  • [x] Improvement upon #176 to use -march=native everywhere except on CI where it seems to cause problems (causing illegal instructions randomly and hard to debug). See also #182
  • [x] Use asset caching to ensure continued operation even if the original source changes or disappears. https://github.com/microsoft/vcpkg/issues/25734#issuecomment-1185969087 or wait for https://github.com/microsoft/vcpkg/pull/29067 to be merged

FabienPean-Virtonomy avatar Dec 02 '22 17:12 FabienPean-Virtonomy

Fabien, I have tried install the new master after your pull request. When making, it show many times "BOOST_HEADER_DEPRECATED", do you have the same issue? Thanks.

Xiangyu-Hu avatar Dec 03 '22 22:12 Xiangyu-Hu

Hi Fabien,

is the building in CI parallelized? It seems that this is at least not explicit.

Xiangyu

Xiangyu-Hu avatar Dec 04 '22 13:12 Xiangyu-Hu

Some tests are two step test, that is, the first step for generating particles, the second for running the simulation. In the new pull request these test are separated to two individual tests . Generally, it is OK. However, as the particle generation is also not exact reproducible. One may face a situation that the particle generation pass but the simulation not. When the CI rerun failed, it will rerun simulation with previous generated particles. In this case, this test may have no way to pass, as it uses the same particle generation which may be the reason for no pass. Therefore, we need the combine the particle generation and simulation into a single test so that both steps are sampled at the same time.

In windows, the combined test can also be done by script.

Xiangyu-Hu avatar Dec 04 '22 18:12 Xiangyu-Hu

is the building in CI parallelized? It seems that this is at least not explicit.

Ninja build system does it in parallel by default.

Some tests are two step test, that is, the first step for generating particles, the second for running the simulation. In the new pull request these test are separated to two individual tests . Generally, it is OK. However, as the particle generation is also not exact reproducible. One may face a situation that the particle generation pass but the simulation not. When the CI rerun failed, it will rerun simulation with previous generated particles. In this case, this test may have no way to pass, as it uses the same particle generation which may be the reason for no pass. Therefore, we need the combine the particle generation and simulation into a single test so that both steps are sampled at the same time.

In windows, the combined test can also be done by script.

If those tests are meant to be run together, then they shouldn't be split in the first place. The source file should be doing both, then we can remove having 2 add_test by a single one.

Fabien, I have tried install the new master after your pull request. When making, it show many times "BOOST_HEADER_DEPRECATED", do you have the same issue? Thanks.

I didn't face it, did you start from a fresh environment? Please make a new issue with the complete log if it is a blocking issue

FabienPean-Virtonomy avatar Dec 05 '22 10:12 FabienPean-Virtonomy

is the building in CI parallelized? It seems that this is at least not explicit.

Ninja build system does it in parallel by default.

Some tests are two step test, that is, the first step for generating particles, the second for running the simulation. In the new pull request these test are separated to two individual tests . Generally, it is OK. However, as the particle generation is also not exact reproducible. One may face a situation that the particle generation pass but the simulation not. When the CI rerun failed, it will rerun simulation with previous generated particles. In this case, this test may have no way to pass, as it uses the same particle generation which may be the reason for no pass. Therefore, we need the combine the particle generation and simulation into a single test so that both steps are sampled at the same time. In windows, the combined test can also be done by script.

If those tests are meant to be run together, then they shouldn't be split in the first place. The source file should be doing both, then we can remove having 2 add_test by a single one.

Fabien, the situation is, in practice these two parts are not always together. it is together only in the test.

Fabien, I have tried install the new master after your pull request. When making, it show many times "BOOST_HEADER_DEPRECATED", do you have the same issue? Thanks.

I didn't face it, did you start from a fresh environment? Please make a new issue with the complete log if it is a blocking issue

Xiangyu-Hu avatar Dec 05 '22 12:12 Xiangyu-Hu

The test is already the same executable with 2 add_test having different program options. It simply means having the test executable running by default (i.e. without program options) generation+timeloop. If a test is meant to test the reload of particles, then we need a dedicated one.

FabienPean-Virtonomy avatar Dec 05 '22 17:12 FabienPean-Virtonomy

The ci is updated already.

Xiangyu-Hu avatar Jul 04 '24 08:07 Xiangyu-Hu