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

Fixing version numbers #1271

Merged
merged 4 commits into from
May 3, 2021
Merged

Fixing version numbers #1271

merged 4 commits into from
May 3, 2021

Conversation

WallyMaier
Copy link
Contributor

@WallyMaier WallyMaier commented Apr 27, 2021

Proposed Changes

This PR fixes the version numbers of files and test cases up to the 7.1.1. (Mostly in NEMO) All files should be consistent now, but I may have missed some.

I am not sure if the test cases were intentionally left old, but I can change that.

EDIT:
After taking a look through, I used my best judgement regarding what is up-to-date. The biggest issue I had was in the sliding_interface sections where configs for Kind_Interpolation= Weight-Average and Nearest-Neighbor. In this case I assumed that these options or used interchangeably among test cases, that I can update both _NN and _WA. I did similar things for some SST vs SA options. Below are the only cases that Im not sure if they are being used/up-to-date with develop:

fea_fsi/SquareCyl_Beam/
incomp_rans/rough_wall/ [pretty sure this is up-to-date, but not used in regressions]
optimization_euler/steady_inverse_design/
sliding_interface/incompressible_unsteady/ [Commented out, so I left this one alone]
sliding_interface/single_stage/
turbomachinery/centrifugal_*/

I also went ahead and removed:
rans/naca0012/_sa_neg [SA_neg is already used in turb_NACA0012_sa.cfg] ?
rans/naca0012/
_sa_binary

Related Work

This is a chore.

PR Checklist

  • [ x] I am submitting my contribution to the develop branch.
  • [ x] My contribution generates no new compiler warnings (try with the '-Wall -Wextra -Wno-unused-parameter -Wno-empty-body' compiler flags, or simply --warnlevel=2 when using meson).
  • [x ] My contribution is commented and consistent with SU2 style.

@WallyMaier
Copy link
Contributor Author

It looks likes my attempts to align the test cases were a failure...tabs :(

I can go through and update those to make it look nice, if everything else looks good.

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.

Are all of these part of the regressions? i.e. do they really run with 7.1.1? Otherwise it would be important to keep the reference with which they are known to run.

@WallyMaier
Copy link
Contributor Author

WallyMaier commented Apr 27, 2021

Sounds good, if the cases are run using the regression I will keep as 7.1.1. I guess this begs the questions of should we be carrying test cases that dont run with the most recent version of the code?

@WallyMaier
Copy link
Contributor Author

I made some comments above regarding the cases.

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.

I guess the answer to that is it depends... ideally all of them would work.

Copy link
Contributor

@TobiKattmann TobiKattmann left a comment

Choose a reason for hiding this comment

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

Otherwise it would be important to keep the reference with which they are known to run.

that would obviously nice ... but during the config option cleanups, options were also removed in cfg's that might just run with older code, i.e. even with the correct version i might fail out of the box.
Thanks @WallyMaier for this 💐

@WallyMaier WallyMaier merged commit 3cc0208 into develop May 3, 2021
@WallyMaier WallyMaier deleted the fix_random_version_numbers branch May 3, 2021 20:07
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