-
Notifications
You must be signed in to change notification settings - Fork 849
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
Minor chores: TURB_MODEL now enum class. #1404
Conversation
Went well for the most part. In some switches a default: break; had to be added. in option_structure.inl the old ENUM_TURB_MODEL was stored in some unsigned short intInfo container which is not seamlessly possible as an enum class names its own new type. Imo the storage is not not necessary as it is never used so I just removed it.
this->intInfo[i][0] = iit->second; | ||
|
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.
As mentioned in the commit message: From my grep-ping I believe this can be safely removed. The intInfo is used for some other WallFunctions to count some amount of nodes which is used in other instances. In this particular instances it stored the enum ENUM_TURB_MODEL which is of course an unsigned short. With enum class that is no longer possible. Therefore I removed it because I think this info is never used
CodeFactor has
|
I wont change the output_structure_legacy though
Now CodeFactor has Complex Code twice and nothing else |
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.
Revert 8f3087b so we can keep some consistency in the code.
"override" is the default for anything virtual, "final" is for special purposes.
…override." This reverts commit 67c1aa9.
Thanks for the reviewing and helpful comments @pcarruscag 👍 I'll merge this once the hybrid_AD passes by chance .. i know that's not strictly necessary but feels better |
Hi all,
I made the ENUM_TURB_MODEL an enum class. I will add a few other small things in here. In case you have some minor cleanup commit feel free to push in here
Also made some turbSA+SST functions final where it was override previously
Related Work
#1220 #1242 and probably some others on the road to enum class
@suargi i hope this will not interfere to heavily with your work on the SA model things
PR Checklist
Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.