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

Add regressions for all convective numerical schemes for NEMO #1885

Merged
merged 11 commits into from
Jan 18, 2023

Conversation

WallyMaier
Copy link
Contributor

I am adding regressions for all the convective numerical schemes in NEMO using the implicit solver. All cases run on an inviscid wedge at the moment, but I am happy to make a new folder in the regression.

Related Work

This will allow us to better track our changes and is the start of a more comprehensive set of regression tests for the Nemo solver coming in the future (boundary conditions, etc.)
This directly relates to #1773 to eventually refactor the AUSM schemes.

PR Checklist

  • 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 have added a test case that demonstrates my contribution, if necessary.
  • [] I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp), if necessary.

@WallyMaier
Copy link
Contributor Author

As mentioned, these all use the same geometry, and I happy to discuss inclusion of different test cases.

Also, I noticed that mutation++ does have a regression case, do we not compile M++ for regression builds?

@WallyMaier WallyMaier changed the title [WIP] Add regressions for all convective numerical schemes for NEMO Add regressions for all convective numerical schemes for NEMO Jan 12, 2023
@WallyMaier WallyMaier requested a review from jtneedels January 12, 2023 18:56
Copy link
Contributor

@jtneedels jtneedels left a comment

Choose a reason for hiding this comment

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

This looks good to me, definitely important to get test coverage on these schemes!

Sounds like all these tested cases run, do you have a comparison of convergence history? I'd be curious if they have similar performance on this test case, and would be good to confirm correct behavior past 10 iters.

@WallyMaier
Copy link
Contributor Author

I do not have a good grasp on the performance of each scheme. I fear that may be a much time-intensive task, particularly since these all are implicit at the moment.

@jtneedels
Copy link
Contributor

I do not have a good grasp on the performance of each scheme. I fear that may be a much time-intensive task, particularly since these all are implicit at the moment.

I'm thinking just run this test case out to convergence for each scheme then comparing convergence histories. That way if any don't converge, we know we're not grandfathering in broken cases/schemes.

@WallyMaier
Copy link
Contributor Author

Here is a quick summary for the inviscid wedge case using different NEMO schemes. There is clearly work to be done in the convergence/robustness world.
nemo_scheme_regressions.pdf

@WallyMaier WallyMaier merged commit 1016eef into develop Jan 18, 2023
@WallyMaier WallyMaier deleted the feature_nemo_ausm_regressions branch January 18, 2023 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants