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 turbomachinery testcases and regression tests #2158

Merged
merged 25 commits into from
Apr 2, 2024

Conversation

alecappiello
Copy link
Contributor

@alecappiello alecappiello commented Nov 3, 2023

Proposed Changes

I just added a new folder in the Turbomachinery test cases, containing the configuration files for the Aachen turbine test case

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson).
  • My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
  • I used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp), if necessary.

@pcarruscag
Copy link
Member

Hi Ale, can you add the mesh (from the workshop I presume) to the Testcases repo with a restart file and then use that to run a coupled iterations for regression testing? We may need to wait for the turbo outputs PR

@alecappiello
Copy link
Contributor Author

Hi Ale, can you add the mesh (from the workshop I presume) to the Testcases repo with a restart file and then use that to run a coupled iterations for regression testing? We may need to wait for the turbo outputs PR

Hi Pedro, I'm adding them now.
Yes, configs and mesh are from the tutorial, although I slightly amended the config to make it work with develop (just one config input commented), so that we don't need to wait for that PR. In fact, I added the folder to develop.

For the regresstion testing (sorry for the silly question, I'm a bit of a novice), is it enough to run the single test case or should I do the proper regression test with provided python script?

@pcarruscag
Copy link
Member

Look for Jones_tc_restart in parallel_regression.py and that should help you set up the regression.

@alecappiello
Copy link
Contributor Author

Look for Jones_tc_restart in parallel_regression.py and that should help you set up the regression.

Thanks for tip!
I've realized that my mesh file is too big to be uploaded as is (about 120 MB). I'll need to make a smaller one, but it'll take me a bit.

@alecappiello
Copy link
Contributor Author

Hi Pedro, I added the regresion tests, and mesh and restart to the Testcases repo (for which I opened a pull request)

@joshkellyjak joshkellyjak changed the title Develop Update turbomachinery testcases and regression tests Nov 8, 2023
@alecappiello alecappiello added documentation Testcase Pull request that introduces a new testcase and removed documentation labels Nov 10, 2023
Removing unused Cauchy convergence criterion
@alecappiello
Copy link
Contributor Author

alecappiello commented Mar 15, 2024

Hi @pcarruscag and @joshkellyjak, the parallel and serial regression tests are successful, but the tsan tests keep failing.
Josh suggested it could be related to the submodules, which I managed to update locally. Though I can't find a way to push it to the fork.
Do you guys have any suggestion?

@pcarruscag
Copy link
Member

@jblueh are the tsan tests using a special version of CoDi?

Note: switching to 'refs/pull/2158/merge'.

You are in 'detached HEAD' state. You can look around, make experimental
M	externals/codi

Does it need some kind of update?

@pcarruscag
Copy link
Member

This branch was green just a couple of day ago https://github.com/su2code/SU2/actions/runs/8264374345

@jblueh
Copy link
Contributor

jblueh commented Mar 15, 2024

@jblueh are the tsan tests using a special version of CoDi?

Note: switching to 'refs/pull/2158/merge'.

You are in 'detached HEAD' state. You can look around, make experimental
M	externals/codi

Does it need some kind of update?

No, CoDi should be fine. Tsan does not need a special version of CoDi and the correct commit is checked out a couple of lines below the quoted ones. Tsan is failing in other PRs, too. There seems to be a general issue with tsan and github runners, I am looking into it right now.

Removing file version from config
Copy link
Member

@pcarruscag pcarruscag left a comment

Choose a reason for hiding this comment

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

Thanks lgtm

@jblueh
Copy link
Contributor

jblueh commented Mar 15, 2024

@alecappiello develop is updated with a fix for the thread sanitizer issue, it should fix your CI pipeline once you merge it into your branch

@alecappiello
Copy link
Contributor Author

@alecappiello develop is updated with a fix for the thread sanitizer issue, it should fix your CI pipeline once you merge it into your branch

Thanks a lot! I've just done it and tsan tests are indeed not failing anymore!

@alecappiello alecappiello merged commit e33871a into su2code:develop Apr 2, 2024
30 of 31 checks passed
@jblueh jblueh mentioned this pull request May 8, 2024
@jblueh jblueh mentioned this pull request May 29, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:chore Testcase Pull request that introduces a new testcase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants