Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
New SA version/correction combinations and new way of specifying them in the config (SA_OPTIONS=...) #1646
New SA version/correction combinations and new way of specifying them in the config (SA_OPTIONS=...) #1646
Changes from 149 commits
ebbf37a
e2887f9
9bd7e91
4344477
bcb6c43
c57f6f5
8d67a08
d8ca557
b5275a1
3252c16
5300b3c
f55c8f4
9118ec0
ffcd49c
7653887
9178a35
aad365e
c71640e
852655e
dac78c8
dcbfb9e
a9c06ea
29909ba
30f59a6
f2cd8c0
34babb7
cd2001c
2da1a82
214b2b1
1df520e
300230f
5669113
4718be2
c74f082
a9271be
469b8ce
7e0b446
d73234d
143c28b
bc9ddcb
49fe8b1
1640f2a
d123868
4df45d5
ea8290f
f3d61c2
41d20ab
df055ed
f893160
3cbeb32
8f8b517
bbc7931
8a751d0
7dddf3b
c112e36
bfa38c5
29d3988
0894bf4
bf312cf
f49c488
d2e763e
df0a997
7ecfe33
1ef95e8
38c3ab5
72c4416
0d24bd0
0224811
3b6616b
445266e
370bbcf
448075a
ca8c998
7fb8e5b
d81913e
aa876c3
32efc68
4b41461
7adf0d9
7f4c663
888a738
dd2f15a
808816f
8c02c31
7f4dd00
d2f05e0
c94264b
d74d5fe
e9f135f
7122b48
cc6054f
7390934
576bfd0
e5ec394
98a3736
8be782a
4dfb8c8
dbcad65
3ad9073
4bc8810
9b2b9ce
17a5e69
35dbc34
958ec0d
7a5ecc8
3572fb5
e7a3092
08b7319
1ea2106
7c2a65c
d0a2009
d6ff7c8
1033583
f21b7a0
8b85d97
c0efb31
fef8828
5dd09c9
7728a57
90d1107
f61026b
34e3865
780fb54
78db549
031740f
2752d7b
95f7fc6
1cc18ba
c890e0a
8b1364c
5c84858
a1d7e57
b08defb
078ffce
f82dc33
e1f6976
cfd1721
73698f7
578121c
28c3fa7
20a20bf
5e07976
57fe61f
5432af6
86c1c01
d5fc713
3341a57
cbe631e
3701aaa
878671d
1a8ff57
3fa7b8a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to use the keyword NOFT2 instead of FT2 to be more compatible with the NASA naming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same problem as with "modified" "unmodified" if we want to default to noft2 we need two options, and one will be redundant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we can change to WITHFT2 to avoid confusion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For compatibility with previous config files and avoid problems with transition, see #1066 (comment), I think it is better to set the default to the no-ft2 version and add activate it with WITHFT2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the nasa website should be leading here. If they call it SA-NOFT2, then I think for the end user it is most clear if we use SA_OPTIONS=NOFT2
Also, if the user sees in the config file that the turbulence model is SA, then it should be SA, and not SA-neg, or SA-noft2.
We should prevent submodels being activated in a hidden way. Maybe it is even better to either force the user to provide an SA_OPTION, to make her aware of the change (my preference), or give a warning that no SA_OPTION is found (might be missed).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would agree with you Nijso, if we were doing it from scratch, but that boat sailed and we are not going to break backward compatibility like that.
We can think about it for V8.
But I have a good solution for the "legal" combinations problem, to use the "illegal" ones it will be necessary to put "EXPERIMENTAL" in the SA options.
Ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, we can postpone the 'breaking' when we have the power of a V8 and the users had some time to adjust.
And if a user really wants to use combinations that are not found in the literature, we can allow it with some warnings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, done!