Create test for the predictor corrector in RTheta geometry. #406

PaulineVidal · 2025-08-06T14:51:13Z

Add a test_diocotron.sh to check if the growth rate of the perturbation of the diocotron instability test case follow the expected slope. It allows us to test the different predictor corrector in the geometryRTheta folder.
Add a time_solver folder and a time_solver/growth_rate_test-py in the tests/geometryRTheta folder.

Remove "Add a periodic_strips_non_uniform_2d_9patches geometry." in the CHANGELOG.md.

Please complete the checklist to ensure that all tasks are completed before marking your pull request as ready for review.

All Submissions

Have you ensured that all lines changed in this PR are justified by a comment found in the description ?
Have you updated the CHANGELOG.md ?
Have you linked any issues that should be closed when this PR is merged (using closing keywords) ?
Have you checked that the AUTHORS file is up to date ?
Have you checked that the copyright information in the LICENCE file is up to date (including dates) ?
Do you follow the conventions specified in our coding standards ?

New Feature Submissions

Have you added tests for the new functionalities ?
Have you documented the new functionalities:
- API documentation describing the available methods, when each should be used and how to use them ?
- User-friendly documentation in README files (which may link to the API documentation).
- If the new functionality is non-trivial to use, provide a tutorial or example ? (optional)

Changes to Existing Features

Have you checked that existing tests cover all code after the changes ?
Have you checked that existing tests are still passing ?
Have you checked that the existing documentation is still accurate (API and README files) ?

Changes to the CI

Have you made the same changes to both the GitHub CI and the GitLab CI (for the private fork) ?

…or_pred_corr

github-actions · 2025-08-06T21:06:17Z

This PR is failing tests so it has been put back into draft. Please remove the draft status when the tests pass.

github-actions · 2025-08-06T21:32:07Z

This PR is failing tests so it has been put back into draft. Please remove the draft status when the tests pass.

tests/geometryRTheta/time_solver/test_diocotron.sh

github-actions · 2025-08-07T10:03:36Z

This PR is failing tests so it has been put back into draft. Please remove the draft status when the tests pass.

github-actions · 2025-08-07T13:33:45Z

This PR is failing tests so it has been put back into draft. Please remove the draft status when the tests pass.

…or_pred_corr

tests/geometryRTheta/time_solver/test_diocotron.sh

…or_pred_corr

EmilyBourne · 2025-08-08T11:36:44Z

tests/geometryRTheta/time_solver/CMakeLists.txt

+            "$<TARGET_FILE:Python3::Interpreter>")
+
+    # The test should take less than 15 min.         
+    set_property(TEST "${test_name}" PROPERTY TIMEOUT 900000)


On the CI the timings are:

Serial Debug GNU

TestSimulationDiocotron_PREDCORR_EULER_METHOD (582.507s)

TestSimulationDiocotron_EXPLICIT_PREDCORR_EULER_METHOD (409.768s)

TestSimulationDiocotron_IMPLICIT_PREDCORR_EULER_METHOD (314.197s)

Serial Debug Clang

TestSimulationDiocotron_PREDCORR_EULER_METHOD (267.755s)

TestSimulationDiocotron_EXPLICIT_PREDCORR_EULER_METHOD (199.956s)

TestSimulationDiocotron_IMPLICIT_PREDCORR_EULER_METHOD (162.804s)

Serial Release GNU

TestSimulationDiocotron_PREDCORR_EULER_METHOD (179.175s)

TestSimulationDiocotron_EXPLICIT_PREDCORR_EULER_METHOD (141.35s)

TestSimulationDiocotron_IMPLICIT_PREDCORR_EULER_METHOD (93.0959s)

Serial Release Clang

TestSimulationDiocotron_PREDCORR_EULER_METHOD (216.769s)

TestSimulationDiocotron_EXPLICIT_PREDCORR_EULER_METHOD (163.87s)

TestSimulationDiocotron_IMPLICIT_PREDCORR_EULER_METHOD (130.083s)

OpenMP Release GNU

TestSimulationDiocotron_PREDCORR_EULER_METHOD (245.865s)

TestSimulationDiocotron_EXPLICIT_PREDCORR_EULER_METHOD (202.417s)

TestSimulationDiocotron_IMPLICIT_PREDCORR_EULER_METHOD (149.567s)

Coverage Flags GNU

TestSimulationDiocotron_PREDCORR_EULER_METHOD (5687.16s)

TestSimulationDiocotron_EXPLICIT_PREDCORR_EULER_METHOD (3960.98s)

TestSimulationDiocotron_IMPLICIT_PREDCORR_EULER_METHOD (2883.25s)

Unacceptably slow

GPU

TestSimulationDiocotron_PREDCORR_EULER_METHOD (68.71s)

TestSimulationDiocotron_EXPLICIT_PREDCORR_EULER_METHOD (73.10s)

TestSimulationDiocotron_IMPLICIT_PREDCORR_EULER_METHOD (519.86s)

Maybe we can choose the parameters based on the CI. Run with 1 step for the coverage just to show there is a test 🤔

It is also probably sufficient to run in Release only if we can work out a clean way to do it

EmilyBourne · 2025-08-08T11:45:03Z

tests/geometryRTheta/time_solver/CMakeLists.txt

+
+    # The test should take less than 15 min.         
+    set_property(TEST "${test_name}" PROPERTY TIMEOUT 900000)
+    set_property(TEST "${test_name}" PROPERTY COST 100)


Suggested change

set_property(TEST "${test_name}" PROPERTY COST 100)

set_property(TEST "${test_name}" PROPERTY COST 100)

set_property(TEST "${test_name}" PROPERY LABEL Release_only)

This would require -L Release_only to be passed to ctest though.

Maybe they could just be activated with an environment variable?

I dont know what is the best solution. The release only is better because indeed we dont need to test in debug.

EmilyBourne · 2025-08-08T11:48:40Z

tests/geometryRTheta/time_solver/CMakeLists.txt

+diocotron_test_executable(PREDCORR EULER_METHOD)
+diocotron_test_executable(EXPLICIT_PREDCORR EULER_METHOD)
+diocotron_test_executable(IMPLICIT_PREDCORR EULER_METHOD)


Suggested change

diocotron_test_executable(PREDCORR EULER_METHOD)

diocotron_test_executable(EXPLICIT_PREDCORR EULER_METHOD)

diocotron_test_executable(IMPLICIT_PREDCORR EULER_METHOD)

if(defined ENV{BUILD_END_TO_END_TESTS})

diocotron_test_executable(PREDCORR EULER_METHOD)

diocotron_test_executable(EXPLICIT_PREDCORR EULER_METHOD)

diocotron_test_executable(IMPLICIT_PREDCORR EULER_METHOD)

endif()

github-actions · 2025-08-08T12:37:24Z

This PR is failing tests so it has been put back into draft. Please remove the draft status when the tests pass.

tests/geometryRTheta/time_solver/CMakeLists.txt

PaulineVidal · 2025-08-08T13:17:34Z

It seems that something is not working with gcov: https://github.com/gyselax/gyselalibxx/actions/runs/16828029669/job/47668971621?pr=406
(but I dont know what it is and the link they give is not working)

EmilyBourne · 2025-08-08T13:19:03Z

It seems that something is not working with gcov: https://github.com/gyselax/gyselalibxx/actions/runs/16828029669/job/47668971621?pr=406 (but I dont know what it is and the link they give is not working)

I'm not certain but it looks like it goes through a section of code so many times (due to the large number of iterations) that it assumes that they forgot to initialise something

EmilyBourne · 2025-08-08T13:20:44Z

Got suspicious hit value in gcov line 8691731840

means it hit this line 8691731840 times. I get the impression that it thinks that if a line of code is run 10^9 times during testing it probably means they measured it wrong.

PaulineVidal · 2025-08-08T13:23:14Z

Got suspicious hit value in gcov line 8691731840

means it hit this line 8691731840 times. I get the impression that it thinks that if a line of code is run 10^9 times during testing it probably means they measured it wrong.

Oh, wow it is a pretty big number 😮
And true, I dont know that kind of part of the code could be hit so many time 🤔

EmilyBourne · 2025-08-08T13:29:19Z

Got suspicious hit value in gcov line 8691731840

means it hit this line 8691731840 times. I get the impression that it thinks that if a line of code is run 10^9 times during testing it probably means they measured it wrong.

Oh, wow it is a pretty big number 😮 And true, I dont know that kind of part of the code could be hit so many time 🤔

It is line 723 of some file. The line contains for (std::size_t j(0); j < ntheta; ++j) {' maybe spline evaluation?

EmilyBourne · 2025-08-08T13:30:08Z

gyselalibxx/src/interpolation/polar_splines/polar_bsplines.hpp

Line 723 in f589ffc

for (std::size_t j(0); j < ntheta; ++j) {

PaulineVidal · 2025-08-08T13:30:20Z

for (std::size_t j(0); j < ntheta; ++j) {

polar_bsplines.hpp

It could be the resolution with Ginkgo? I know that to invert the matrix it iterate several times. With not a well refined gird, maybe it is more difficult to invert?

EmilyBourne · 2025-08-08T13:33:34Z

for (std::size_t j(0); j < ntheta; ++j) {

polar_bsplines.hpp

It could be the resolution with Ginkgo?

Ginkgo can't call this function. In the Poisson solver it is used (a lot but not 10^9 a lot) for the assembly and it is used to build the RHS (NQuadPts * Nr * Ntheta * 4(cells) * 2(func and basis in weak formulation) max per call to Poisson::operator())

EmilyBourne · 2025-08-08T13:35:27Z

for (std::size_t j(0); j < ntheta; ++j) {

polar_bsplines.hpp
It could be the resolution with Ginkgo?

Ginkgo can't call this function. In the Poisson solver it is used (a lot but not 10^9 a lot) for the assembly and it is used to build the RHS (NQuadPts * Nr * Ntheta * 4(cells) * 2(func and basis in weak formulation) max per call to Poisson::operator())

This could plausibly be ~10^6 calls so 10^9 is not out of reach if you then have 1000 time steps

PaulineVidal · 2025-08-08T13:39:12Z

for (std::size_t j(0); j < ntheta; ++j) {

polar_bsplines.hpp
It could be the resolution with Ginkgo?

Ginkgo can't call this function. In the Poisson solver it is used (a lot but not 10^9 a lot) for the assembly and it is used to build the RHS (NQuadPts * Nr * Ntheta * 4(cells) * 2(func and basis in weak formulation) max per call to Poisson::operator())

This could plausibly be ~10^6 calls so 10^9 is not out of reach if you then have 1000 time steps

Mmh, indeed the diocotron by default has 700 time steps. Here I reduced it to 400 times steps. But with the predcorr the operators can be call twice (prediction and then correction) or even more for the implicit predcorr!

PaulineVidal added 2 commits August 6, 2025 14:57

start to add a test for predcorr

c710a3e

Google test to modify

99c5a95

PaulineVidal had a problem deploying to GitLab GPU trigger August 6, 2025 14:54 — with GitHub Actions Error

EmilyBourne marked this pull request as draft August 6, 2025 15:07

PaulineVidal added 3 commits August 6, 2025 22:58

add files for test

de830a2

Merge remote-tracking branch 'origin/devel' into pvidal_google_test_f…

aec4974

…or_pred_corr

remove comment

4e901fe

PaulineVidal marked this pull request as ready for review August 6, 2025 21:05

github-actions bot marked this pull request as draft August 6, 2025 21:06

line

30c0886

PaulineVidal marked this pull request as ready for review August 6, 2025 21:07

PaulineVidal had a problem deploying to GitLab GPU trigger August 6, 2025 21:09 — with GitHub Actions Error

github-actions bot marked this pull request as draft August 6, 2025 21:32

EmilyBourne reviewed Aug 7, 2025

View reviewed changes

tests/geometryRTheta/time_solver/test_diocotron.sh Outdated Show resolved Hide resolved

on local it doesnt work. Try on Persee.

130839f

PaulineVidal marked this pull request as ready for review August 7, 2025 09:38

PaulineVidal had a problem deploying to GitLab GPU trigger August 7, 2025 09:40 — with GitHub Actions Error

remove useless file

6413737

github-actions bot marked this pull request as draft August 7, 2025 10:03

PaulineVidal marked this pull request as ready for review August 7, 2025 10:05

PaulineVidal had a problem deploying to GitLab GPU trigger August 7, 2025 10:06 — with GitHub Actions Error

github-actions bot marked this pull request as draft August 7, 2025 13:33

change the parameter in the .yaml

2936963

PaulineVidal marked this pull request as ready for review August 7, 2025 14:21

PaulineVidal had a problem deploying to GitLab GPU trigger August 7, 2025 14:22 — with GitHub Actions Error

Merge remote-tracking branch 'origin/devel' into pvidal_google_test_f…

1c82ca7

…or_pred_corr

EmilyBourne reviewed Aug 8, 2025

View reviewed changes

tests/geometryRTheta/time_solver/test_diocotron.sh Outdated Show resolved Hide resolved

change path

561d734

PaulineVidal had a problem deploying to GitLab GPU trigger August 8, 2025 07:50 — with GitHub Actions Error

PaulineVidal marked this pull request as draft August 8, 2025 07:51

PaulineVidal added 3 commits August 8, 2025 12:16

clean files

7c2446f

Merge remote-tracking branch 'origin/devel' into pvidal_google_test_f…

1116ac0

…or_pred_corr

remove line in the CHANGELOG which shouldnt be here.

db59cec

PaulineVidal marked this pull request as ready for review August 8, 2025 10:22

PaulineVidal temporarily deployed to GitLab GPU trigger August 8, 2025 10:22 — with GitHub Actions Inactive

EmilyBourne reviewed Aug 8, 2025

View reviewed changes

github-actions bot marked this pull request as draft August 8, 2025 12:37

EmilyBourne reviewed Aug 8, 2025

View reviewed changes

tests/geometryRTheta/time_solver/CMakeLists.txt Outdated Show resolved Hide resolved

PaulineVidal added 3 commits August 8, 2025 15:08

add if with BUILD_END_TO_END_TESTS

f7e2bad

add release only

1fb0d08

TIMEOUT is in s.

61376c7

PROPERY is not the new PROPERTY

da9a05a

EmilyBourne self-assigned this Aug 11, 2025

	set_property(TEST "${test_name}" PROPERTY COST 100)
	set_property(TEST "${test_name}" PROPERTY COST 100)
	set_property(TEST "${test_name}" PROPERY LABEL Release_only)

Create test for the predictor corrector in RTheta geometry. #406

Are you sure you want to change the base?

Create test for the predictor corrector in RTheta geometry. #406

Uh oh!

Conversation

PaulineVidal commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

All Submissions

New Feature Submissions

Changes to Existing Features

Changes to the CI

Uh oh!

github-actions bot commented Aug 6, 2025

Uh oh!

github-actions bot commented Aug 6, 2025

Uh oh!

Uh oh!

github-actions bot commented Aug 7, 2025

Uh oh!

github-actions bot commented Aug 7, 2025

Uh oh!

Uh oh!

EmilyBourne Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EmilyBourne Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

EmilyBourne Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

EmilyBourne Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

EmilyBourne Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

EmilyBourne Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

PaulineVidal Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EmilyBourne Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 8, 2025

Uh oh!

Uh oh!

PaulineVidal commented Aug 8, 2025

Uh oh!

EmilyBourne commented Aug 8, 2025

Uh oh!

EmilyBourne commented Aug 8, 2025

Uh oh!

PaulineVidal commented Aug 8, 2025

Uh oh!

EmilyBourne commented Aug 8, 2025

Uh oh!

EmilyBourne commented Aug 8, 2025

Uh oh!

PaulineVidal commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EmilyBourne commented Aug 8, 2025

Uh oh!

EmilyBourne commented Aug 8, 2025

Uh oh!

PaulineVidal commented Aug 8, 2025

Uh oh!

Uh oh!

PaulineVidal commented Aug 6, 2025 •

edited

Loading

EmilyBourne Aug 8, 2025 •

edited

Loading

PaulineVidal Aug 8, 2025 •

edited

Loading

PaulineVidal commented Aug 8, 2025 •

edited

Loading