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

Reduce testing time for parablic tests #1632

Closed
sloede opened this issue Sep 12, 2023 · 5 comments · Fixed by #1634
Closed

Reduce testing time for parablic tests #1632

sloede opened this issue Sep 12, 2023 · 5 comments · Fixed by #1634
Labels

Comments

@sloede
Copy link
Member

sloede commented Sep 12, 2023

The test TreeMesh3D: elixir_navierstokes_taylor_green_vortex.jl (Refined mesh) runs for 2000 steps and takes > 10 minutes in CI, see, e.g. here. We should reduce this to more reasonable numbers (maybe 200 steps?) to help keeping the test duration reasonable.

@DanielDoehring could you maybe update the test setup in test/test_parabolic_3d.jl? I also had a look at the other tests in the same file to see if there are other tests that could benefit from pruning but didn't find any; maybe you can double check just to be sure I didn't miss anything.

@sloede sloede added the testing label Sep 12, 2023
@DanielDoehring
Copy link
Contributor

DanielDoehring commented Sep 12, 2023

TreeMesh3D: elixir_navierstokes_taylor_green_vortex.jl (Refined mesh) has been added in 3f48f64 - no objections from my side to shorten the test duration.

Concerning the second part: Sure, I will take a look if there is something that may be combined.

@sloede
Copy link
Member Author

sloede commented Sep 12, 2023

I am sorry, was I wrong to ping you for this? I had a look at the blame in https://github.com/trixi-framework/Trixi.jl/blame/3523c49120d7c282518769a5b3d40ce7c9cc5882/test/test_parabolic_3d.jl#L110-L143 and thought that the refined test was added in your PR, that's why I asked. But I'm sorry if I mistook you for the author!

If you could have a look anyways, that would be great! And I didn't mean to combine tests, I just meant to check that there are no other tests with "long sounding" tspans like this one. No need to restructure anything, just maybe shorten other tests that run for significantly more than 2 mins.

@DanielDoehring
Copy link
Contributor

I am sorry, was I wrong to ping you for this? I had a look at the blame in https://github.com/trixi-framework/Trixi.jl/blame/3523c49120d7c282518769a5b3d40ce7c9cc5882/test/test_parabolic_3d.jl#L110-L143 and thought that the refined test was added in your PR, that's why I asked. But I'm sorry if I mistook you for the author!

Ah got it, sorry, yeah that is a test I added when the mortars where added. I believe the test time was chosen that long to test for allocations.

In principle, one could also delete the tests which I added for the mortars, i.e., the tests which manually call refine! since this is now part of the AMR tests anyway.

@sloede
Copy link
Member Author

sloede commented Sep 12, 2023

In principle, one could also delete the tests which I added for the mortars, i.e., the tests which manually call refine! since this is now part of the AMR tests anyway.

I don't have all the details, but it's your code, so you probably know best 🤷‍♂️ It might be nice to keep one test with refine! around to have a blueprint for people who want to manually refine their mesh (but it might already exist elsewhere, so just use your judgement)

@DanielDoehring
Copy link
Contributor

It might be nice to keep one test with refine! around to have a blueprint for people who want to manually refine their mesh (but it might already exist elsewhere, so just use your judgement)

I agree, the examples showcase somewhat nicely how one can alter the setup in an elixir without adding one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants