Skip to content
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

Update restart files of application tests for p4est 2.3.6 #1181

Merged
merged 14 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ See [this page](https://chaos-polymtl.github.io/lethe/documentation/contributing
Code related list:
- [ ] All in-code documentation related to this PR is up to date (Doxygen format)
- [ ] Lethe documentation is up to date
- [ ] Fix has unit test(s) (preferred) or application test(s)
- [ ] Fix has unit test(s) (preferred) or application test(s), and restart files are in the generator folder
- [ ] The branch is rebased onto master
- [ ] Changelog (CHANGELOG.md) is up to date
- [ ] Code is indented with indent-all and .prm files (examples and tests) with prm-indent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ See [this page](https://chaos-polymtl.github.io/lethe/documentation/contributing
Code related list:
- [ ] All in-code documentation related to this PR is up to date (Doxygen format)
- [ ] Lethe documentation is up to date
- [ ] New feature has unit test(s) (preferred) or application test(s)
- [ ] New feature has unit test(s) (preferred) or application test(s), and restart files are in the generator folder
- [ ] The branch is rebased onto master
- [ ] Changelog (CHANGELOG.md) is up to date
- [ ] Code is indented with indent-all and .prm files (examples and tests) with prm-indent
Expand Down
11 changes: 9 additions & 2 deletions .github/workflows/main_release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ jobs:
make -j${{ env.COMPILE_JOBS }}
# These tests require a single core each so we will run them in parallel
# Only run tests on deal.ii master version
# Restart files are not compatible with deal.ii v9.5.0
- name: Run Lethe tests (Release-deal.ii:${{ matrix.dealii_version }})
if: ${{ matrix.dealii_version == 'master'}}
run: |
#Allow OMPI to run as root
export OMPI_ALLOW_RUN_AS_ROOT=1
Expand All @@ -73,9 +76,13 @@ jobs:
ctest -N --exclude-regex ${{ env.MULTI_CORE_TESTS_REGEX }}
# Run in parallel
ctest --output-on-failure -j${{ env.COMPILE_JOBS }} --exclude-regex ${{ env.MULTI_CORE_TESTS_REGEX }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add an error when matrix.dealii_version != 'master' for clarity.
Something like:

Suggested change
if: ${{ matrix.dealii_version == 'master'}}
run: ...
else:
error:
message: "Version of deal.ii for tests is not the same as their master image."
code: 1
details: "Restart files are not compatible with deal.ii v9.5.0"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a cool suggestion.... as long as that does not throw an error I am down for that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a documentation of the else statement and error? I don't find it in the GitHub action documentation.
Also, giving an error won't failed the check?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it gives an error it would indeed fail the check.
To keep the PR finalized because it's a critical one, let's postpone this check to a next PR. We can open an issue ot keep track of this idea, but to be honest, right now just writing a comment is all good for the time being. Throwing an error would break the CI and so code: 1 would most likely give an error.
Let's postpone that for the future

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I don't change anything at the moment?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, let's not change anything. Let's merge the PR as-is when you are ready with the current feature set. Then if we want to change other stuff, let's make small PRs for them

# These tests require two cores each so we will run them sequencially
# Only run tests on deal.ii master version
# Restart files are not compatible with deal.ii v9.5.0
- name: Run multi-core Lethe tests (Release-deal.ii:${{ matrix.dealii_version }})
if: ${{ matrix.dealii_version == 'master'}}
run: |
#Allow OMPI to run as root
export OMPI_ALLOW_RUN_AS_ROOT=1
Expand All @@ -84,4 +91,4 @@ jobs:
# Print the tests to be executed
ctest -N --tests-regex ${{ env.MULTI_CORE_TESTS_REGEX }}
# Run sequencially
ctest --output-on-failure --tests-regex ${{ env.MULTI_CORE_TESTS_REGEX }}
ctest --output-on-failure --tests-regex ${{ env.MULTI_CORE_TESTS_REGEX }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add the same else error as the previous comment here.

7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@
All notable changes to the Lethe project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/).

## [Master] - 2024-06-16

### Changed

- MINOR Some application-test restart files have been updated using p4est 2.3.6. Some test results have changed for lethe-fluid-particles and lethe-fluid-vans, since the DEM solver has slightly changed since the previous restart files generation, and it is now impossible to regenerate the exact same initial condition. [#1181](https://github.com/chaos-polymtl/lethe/pull/1181)


## [Master] - 2024-06-16

### Changed
Expand Down
7 changes: 0 additions & 7 deletions applications_tests/lethe-fluid-particles/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,6 @@ file(COPY particle_sedimentation_files/dem.triangulation.info DESTINATION "${CMA
file(COPY particle_sedimentation_files/dem.triangulation_fixed.data DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/particle_sedimentation.${_build_type}/mpirun=1/")
file(COPY particle_sedimentation_files/dem.triangulation_variable.data DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/particle_sedimentation.${_build_type}/mpirun=1/")

acdaigneault marked this conversation as resolved.
Show resolved Hide resolved
file(COPY restart_particle_sedimentation_files/dem.particles DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/restart_particle_sedimentation.${_build_type}/mpirun=1/")
file(COPY restart_particle_sedimentation_files/dem.pvdhandler DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/restart_particle_sedimentation.${_build_type}/mpirun=1/")
file(COPY restart_particle_sedimentation_files/dem.simulationcontrol DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/restart_particle_sedimentation.${_build_type}/mpirun=1/")
file(COPY restart_particle_sedimentation_files/dem.triangulation DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/restart_particle_sedimentation.${_build_type}/mpirun=1/")
file(COPY restart_particle_sedimentation_files/dem.triangulation.info DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/restart_particle_sedimentation.${_build_type}/mpirun=1/")
file(COPY restart_particle_sedimentation_files/dem.triangulation_fixed.data DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/restart_particle_sedimentation.${_build_type}/mpirun=1/")
file(COPY restart_particle_sedimentation_files/dem.triangulation_variable.data DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/restart_particle_sedimentation.${_build_type}/mpirun=1/")
file(COPY restart_particle_sedimentation_files/case.particles DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/restart_particle_sedimentation.${_build_type}/mpirun=1/")
file(COPY restart_particle_sedimentation_files/case.pvdhandler DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/restart_particle_sedimentation.${_build_type}/mpirun=1/")
file(COPY restart_particle_sedimentation_files/case_particles.pvdhandler DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/restart_particle_sedimentation.${_build_type}/mpirun=1/")
Expand Down
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the new p4est version cause the changes in the application_tests? Or did the tests have been changed change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case I'm not sure, but I don't think so. A few things have changed in the DEM solver that causes tiny differences. The mobility status can change in one iteration.
I build this test and it was pretty hard to get all the status with a very small number of particles. The test is probably very sensitive to the initial condition. Or the generation files in the repo were not the good ones.

Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ DEM contact search at dem step 0
DEM contact search at dem step 1
Finished 50 DEM iterations
---------------------------------------------------------------
Global continuity equation error: 3.13678e-09 s^-1
Max local continuity error: 6.13917e-08 s^-1
Global continuity equation error: 2.803e-09 s^-1
Max local continuity error: 7.07295e-08 s^-1

**********************************************************************************
Transient iteration: 2 Time: 0.002 Time step: 0.001 CFL: 7.47157e-05
Transient iteration: 2 Time: 0.002 Time step: 0.001 CFL: 7.48857e-05
**********************************************************************************
--------------
Void Fraction
Expand All @@ -50,11 +50,11 @@ DEM
DEM contact search at dem step 1
Finished 50 DEM iterations
---------------------------------------------------------------
Global continuity equation error: -7.56756e-09 s^-1
Max local continuity error: 3.10857e-06 s^-1
Global continuity equation error: -1.26904e-09 s^-1
Max local continuity error: 7.28588e-07 s^-1

**********************************************************************************
Transient iteration: 3 Time: 0.003 Time step: 0.001 CFL: 0.000162479
Transient iteration: 3 Time: 0.003 Time step: 0.001 CFL: 0.000149645
**********************************************************************************
--------------
Void Fraction
Expand All @@ -68,11 +68,11 @@ DEM
DEM contact search at dem step 1
Finished 50 DEM iterations
---------------------------------------------------------------
Global continuity equation error: 1.94709e-09 s^-1
Max local continuity error: 5.08953e-07 s^-1
Global continuity equation error: 3.9841e-09 s^-1
Max local continuity error: 1.05602e-06 s^-1

**********************************************************************************
Transient iteration: 4 Time: 0.004 Time step: 0.001 CFL: 0.000224045
Transient iteration: 4 Time: 0.004 Time step: 0.001 CFL: 0.000224447
**********************************************************************************
--------------
Void Fraction
Expand All @@ -86,12 +86,12 @@ DEM
DEM contact search at dem step 1
Finished 50 DEM iterations
---------------------------------------------------------------
Global continuity equation error: 7.6613e-09 s^-1
Max local continuity error: 5.17523e-07 s^-1
Global continuity equation error: 1.16336e-09 s^-1
Max local continuity error: 1.41713e-06 s^-1

*********************************************************************************
Transient iteration: 5 Time: 0.005 Time step: 0.001 CFL: 0.00029829
*********************************************************************************
**********************************************************************************
Transient iteration: 5 Time: 0.005 Time step: 0.001 CFL: 0.000299344
**********************************************************************************
--------------
Void Fraction
--------------
Expand All @@ -107,7 +107,7 @@ print_from_processor_0
0 1
0
1 8
3 3 3 3 3 3 3 3
4 4 4 4 4 4 4 4


[deal.II intermediate Patch<3,3>]
Expand All @@ -117,7 +117,7 @@ print_from_processor_0
1 1
0
1 8
2 2 2 2 2 2 2 2
4 4 4 4 4 4 4 4


[deal.II intermediate Patch<3,3>]
Expand All @@ -127,7 +127,7 @@ print_from_processor_0
2 1
0
1 8
2 2 2 2 2 2 2 2
4 4 4 4 4 4 4 4


[deal.II intermediate Patch<3,3>]
Expand All @@ -137,7 +137,7 @@ print_from_processor_0
3 1
0
1 8
2 2 2 2 2 2 2 2
4 4 4 4 4 4 4 4


[deal.II intermediate Patch<3,3>]
Expand All @@ -147,7 +147,7 @@ print_from_processor_0
4 1
0
1 8
3 3 3 3 3 3 3 3
4 4 4 4 4 4 4 4


[deal.II intermediate Patch<3,3>]
Expand Down Expand Up @@ -188,5 +188,5 @@ DEM
DEM contact search at dem step 1
Finished 50 DEM iterations
---------------------------------------------------------------
Global continuity equation error: 6.76463e-10 s^-1
Max local continuity error: 6.79928e-07 s^-1
Global continuity equation error: -8.07686e-10 s^-1
Max local continuity error: 1.81029e-06 s^-1
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0 0 350 74 350
0 0 350 78 350
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
0
Time File
Time File
Binary file not shown.
Binary file not shown.
Binary file not shown.
Loading
Loading